feat(baseball): sunset the bundled scroll fallback, standardise on the core - #350
Conversation
…e core The third B6 sunset, after hockey (#346) and football (#349). Same shape: scroll_display_legacy.py deleted, the guarded import of the core's src.common.sports_scroll collapsed to a plain one, manifest floored at 3.2.0. Keeping the try/except with nothing behind it would name the missing scroll_display_legacy rather than the core module actually absent, which is the single log line a user gets before the scoreboard stops appearing. 764 lines removed. Baseball is the one plugin whose frozen copy carried orchestration logic the core does not have, and that is the point of doing it deliberately rather than by deletion. Its _configure_scroll_helper had an extra branch: when scroll_speed * scroll_delay fell outside the 0.1-5.0 pixels-per-frame window but scroll_speed alone sat inside it, it reinterpreted scroll_speed as pixels-per-FRAME rather than the pixels-per-second the setting is documented as. Verified by running both implementations: scroll_speed delay legacy core 50.0 0.01 0.5 0.5 (the default -- agree) 1.0 0.01 1.0 0.1 10x 2.0 0.01 2.0 0.1 20x 0.5 0.01 0.5 0.1 5x Checked across all eight lineages: baseball's was the only copy with it. The core's behaviour is the one to keep. The branch silently ignored the unit the setting is defined in and ran an order of magnitude faster than asked; the core honours the configured pixels-per-second and clamps to the same window, which is what every other scoreboard already does. Re-measured after the collapse: baseball now matches the core exactly at every point in the range above. In practice this reaches nobody. The branch only ever ran on a pre-3.2.0 core; everyone on 3.2.0 or newer has been on the core path since 1.22.0, and the new floor stops this version reaching the rest. Recorded in the manifest anyway, because a silently retired behaviour is worse than a documented one. test_core_fallback.py -> test_core_scroll.py, the same rewrite hockey and football got. SUNSET_PLUGINS grows to three. Verified: all three separator-icon constants and every method survive the de-indent byte-for-byte (MLB_SEPARATOR_ICON, MILB_SEPARATOR_ICON, NCAA_BASEBALL_SEPARATOR_ICON, SCROLL_LEAGUE_KEYS, _SCHEMA_CARD_WIDTH); 24 of 24 safety-harness renders pass across eight panel sizes; the four repo gates pass; baseball's suite is 26 passed, 0 skipped, 0 failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 | 86 |
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.
…#351) * feat(sports): sunset the bundled scroll fallback in the last five afl, basketball, lacrosse, nrl and soccer. Same shape as hockey (#346), football (#349) and baseball (#350): scroll_display_legacy.py deleted, the guarded import of the core's src.common.sports_scroll collapsed to a plain one, manifests floored at 3.2.0. 3,622 lines of frozen copy removed. Checked before deleting, not after. Every orchestration method in all five frozen copies was diffed against the core's, looking for logic the core lacks: - `if not self.scroll_helper` guards -- unreachable in core, which imports ScrollHelper unguarded and always constructs one. Legacy needed them because its own import was guarded and it sets self.scroll_helper = None. - `get_dynamic_duration`'s `return 60` fallback -- same unreachable guard. - `_scroll_start_time` -- legacy reads it, core does not, but only to compute an average-FPS debug line that core produces from _fps_sample_start instead. - `get_current_leagues` returning `.copy()` vs `list()` -- identical. - `_log_scroll_progress` throttling -- core has it. - `clear()` -- core resets strictly more state. None is a behaviour the core is missing. Baseball's px/frame heuristic was the only real one across all eight, and it was handled in #350. Two tests were relying on the guard, and both are worth naming because the sunset is what exposed them: - lacrosse/test_lacrosse_plugin.py stubs the host `src` modules so the plugin imports without a core, and the list did not include src.common.sports_scroll -- the guard used to swallow that. Stubbed now, with real classes rather than None, since ScrollDisplay subclasses one at module level. - soccer/test_live_screens.py installs a stub `src` package to fake src.logo_downloader, which SHADOWED the core. So its guarded import had been falling back, and the test has been exercising the frozen copy rather than the class that ships -- since B5. The stub now carries a __path__ into the real core so only logo_downloader is faked. Driving the real class then surfaced a missing display_width on its hand-built object, which the legacy path never read. Verified: every method and class constant survives the de-indent byte-for-byte in all five, separator icons included; 112 safety-harness renders pass (24 each for afl, basketball, nrl and soccer, 16 for lacrosse); five repo gates pass; fleet is 225 passed, 2 skipped, 0 failed. SUNSET_PLUGINS names seven. Hockey is the eighth and its sunset (#346) merged into #333 rather than main, so it arrives with that branch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 * feat(hockey): sunset the bundled scroll fallback, completing B6 Hockey joins this PR so it is not stranded. Its sunset previously lived only in #346, which merged into #333's branch rather than main -- and #333 is superseded by #353, which does not carry the sunset. Closing #333 without this would leave hockey the one scoreboard of eight still shipping a fallback. Same change as the other seven: scroll_display_legacy.py deleted (703 lines), the guarded import collapsed to a plain one, floor raised to 3.2.0. test_core_fallback.py -> test_core_scroll.py, identical to afl's but for the Run: path. Bumped to 1.22.0 rather than 1.21.0 so it clears every version hockey currently holds anywhere: 1.20.3 on main, 1.21.0 on #353's branch. A floor is only meaningful on a version that can actually supersede what users have. SUNSET_PLUGINS now names all eight, which is the point of listing it rather than inferring it -- the set is a statement that the sunset holds, and it is now true of the whole fleet. Also brings docs/plugin-development/08-shared-sports-code.md up to date. Its sunset rule still read "Until condition 3 holds, keep the guarded try-core/except-local import", which was correct in August and is now the opposite of what the fleet does. Condition 3 holds for src.common.sports_scroll: the store refuses on all three routes in -- install_plugin (core #431/#433), the git-pull branch of update_plugin (#508) and install_from_url (#510). The rule now says to keep the guard for modules that have NOT been through a sunset, to drop it along with the copy for those that have, and to raise the floor in the same commit as the deletion. The instruction to keep it in step with the core doc "in the same PR" is corrected too: they are in different repositories, so that was never possible. Verified: all four separator-icon constants and every method survive the de-indent byte-for-byte; hockey 19 passed, 0 failed; 16 safety-harness renders pass; five repo gates pass, with check_scroll_adoption now reporting 8 sunset plugins free of a fallback; fleet 225 passed, 2 skipped, 0 failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 --------- Co-authored-by: Claude <noreply@anthropic.com>
* feat(football): sunset the bundled scroll fallback, floor at 3.2.0
scroll_display_legacy.py is deleted and the guarded import of the core's
src.common.sports_scroll collapses to a plain one, with the manifest floored at
3.2.0 to match. Keeping the try/except with nothing behind it would name the
missing scroll_display_legacy rather than the core module actually absent,
which is the single log line a user gets before the scoreboard stops appearing.
709 lines removed.
Football is the one plugin where this is not a pure deletion, and that deserved
measuring rather than asserting. Everywhere else the frozen copy and the live
path are method-for-method identical; here _default_game_card_width diverged.
The frozen one returns max(128, display_height * 2 + 40); the adopted one
measures the score gap with a throwaway GameRenderer and converges, which is
what stopped the score being drawn across the logos on tall cards.
Built both classes and compared across every supported panel:
panel legacy core delta
64x32 128 128 same
128x32 128 128 same
256x32 128 128 same
64x64 168 176 +8
128x64 168 176 +8
256x64 168 176 +8
128x96 232 240 +8
256x128 296 304 +8
Identical on every 32-tall panel, 8px wider on taller ones, in classic and
adaptive layout alike.
That difference only ever reached users on a pre-3.2.0 core, because everyone
on 3.2.0 or newer has been on the measured path since 2.29.0 -- the fallback
was never the modern path's behaviour. And those users keep the version they
have, since the new floor stops this one reaching them. So the population that
could observe the change is exactly the population that will not receive it: no
board changes what it draws. Recorded in the manifest notes anyway, because
"removed dead code" would be false and the next person deserves the real
answer.
test_core_fallback.py -> test_core_scroll.py, the same rewrite hockey got in
#346 (the two files were byte-identical but for the Run: path, so this is that
rewrite with one substitution). It asserts the sunset rather than the fallback:
the import is top-level and unguarded, no copy exists or is imported, the base
is the core class by identity, an old core fails naming exactly
src.common.sports_scroll, and the manifest floors at 3.2.0 or above.
check_scroll_adoption.py gains sunset_violations and SUNSET_PLUGINS, the same
gate #346 adds for hockey. offending_classes is left byte-identical so its
eleven pinned cases keep meaning what they mean. The existing check asks
whether the fallback was INLINED and structurally cannot ask whether it still
EXISTS -- it opens scroll_display.py and nothing else -- so a resurrected file
or a returned guard would both pass it silently. #346 and this PR each name
their own plugin; whichever lands second resolves a one-line conflict in the
set.
Verified: separator-icon constants and every method survive the de-indent
byte-for-byte (NFL_SEPARATOR_ICON, NCAA_FB_SEPARATOR_ICON, SCROLL_LEAGUE_KEYS,
_SCHEMA_CARD_WIDTH); 24 of 24 safety-harness renders pass across eight panel
sizes and three modes; the four repo gates pass; football's suite is 40 passed,
1 skipped, 0 failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9
* feat(baseball): sunset the bundled scroll fallback, standardise on the core
The third B6 sunset, after hockey (#346) and football (#349). Same shape:
scroll_display_legacy.py deleted, the guarded import of the core's
src.common.sports_scroll collapsed to a plain one, manifest floored at 3.2.0.
Keeping the try/except with nothing behind it would name the missing
scroll_display_legacy rather than the core module actually absent, which is the
single log line a user gets before the scoreboard stops appearing. 764 lines
removed.
Baseball is the one plugin whose frozen copy carried orchestration logic the
core does not have, and that is the point of doing it deliberately rather than
by deletion. Its _configure_scroll_helper had an extra branch: when
scroll_speed * scroll_delay fell outside the 0.1-5.0 pixels-per-frame window
but scroll_speed alone sat inside it, it reinterpreted scroll_speed as
pixels-per-FRAME rather than the pixels-per-second the setting is documented
as. Verified by running both implementations:
scroll_speed delay legacy core
50.0 0.01 0.5 0.5 (the default -- agree)
1.0 0.01 1.0 0.1 10x
2.0 0.01 2.0 0.1 20x
0.5 0.01 0.5 0.1 5x
Checked across all eight lineages: baseball's was the only copy with it.
The core's behaviour is the one to keep. The branch silently ignored the unit
the setting is defined in and ran an order of magnitude faster than asked; the
core honours the configured pixels-per-second and clamps to the same window,
which is what every other scoreboard already does. Re-measured after the
collapse: baseball now matches the core exactly at every point in the range
above.
In practice this reaches nobody. The branch only ever ran on a pre-3.2.0 core;
everyone on 3.2.0 or newer has been on the core path since 1.22.0, and the new
floor stops this version reaching the rest. Recorded in the manifest anyway,
because a silently retired behaviour is worse than a documented one.
test_core_fallback.py -> test_core_scroll.py, the same rewrite hockey and
football got. SUNSET_PLUGINS grows to three.
Verified: all three separator-icon constants and every method survive the
de-indent byte-for-byte (MLB_SEPARATOR_ICON, MILB_SEPARATOR_ICON,
NCAA_BASEBALL_SEPARATOR_ICON, SCROLL_LEAGUE_KEYS, _SCHEMA_CARD_WIDTH); 24 of 24
safety-harness renders pass across eight panel sizes; the four repo gates pass;
baseball's suite is 26 passed, 0 skipped, 0 failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9
* feat(sports): sunset the bundled scroll fallback in the remaining six (#351)
* feat(sports): sunset the bundled scroll fallback in the last five
afl, basketball, lacrosse, nrl and soccer. Same shape as hockey (#346),
football (#349) and baseball (#350): scroll_display_legacy.py deleted, the
guarded import of the core's src.common.sports_scroll collapsed to a plain one,
manifests floored at 3.2.0. 3,622 lines of frozen copy removed.
Checked before deleting, not after. Every orchestration method in all five
frozen copies was diffed against the core's, looking for logic the core lacks:
- `if not self.scroll_helper` guards -- unreachable in core, which imports
ScrollHelper unguarded and always constructs one. Legacy needed them because
its own import was guarded and it sets self.scroll_helper = None.
- `get_dynamic_duration`'s `return 60` fallback -- same unreachable guard.
- `_scroll_start_time` -- legacy reads it, core does not, but only to compute
an average-FPS debug line that core produces from _fps_sample_start instead.
- `get_current_leagues` returning `.copy()` vs `list()` -- identical.
- `_log_scroll_progress` throttling -- core has it.
- `clear()` -- core resets strictly more state.
None is a behaviour the core is missing. Baseball's px/frame heuristic was the
only real one across all eight, and it was handled in #350.
Two tests were relying on the guard, and both are worth naming because the
sunset is what exposed them:
- lacrosse/test_lacrosse_plugin.py stubs the host `src` modules so the plugin
imports without a core, and the list did not include src.common.sports_scroll
-- the guard used to swallow that. Stubbed now, with real classes rather than
None, since ScrollDisplay subclasses one at module level.
- soccer/test_live_screens.py installs a stub `src` package to fake
src.logo_downloader, which SHADOWED the core. So its guarded import had been
falling back, and the test has been exercising the frozen copy rather than
the class that ships -- since B5. The stub now carries a __path__ into the
real core so only logo_downloader is faked. Driving the real class then
surfaced a missing display_width on its hand-built object, which the legacy
path never read.
Verified: every method and class constant survives the de-indent byte-for-byte
in all five, separator icons included; 112 safety-harness renders pass (24 each
for afl, basketball, nrl and soccer, 16 for lacrosse); five repo gates pass;
fleet is 225 passed, 2 skipped, 0 failed.
SUNSET_PLUGINS names seven. Hockey is the eighth and its sunset (#346) merged
into #333 rather than main, so it arrives with that branch.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9
* feat(hockey): sunset the bundled scroll fallback, completing B6
Hockey joins this PR so it is not stranded. Its sunset previously lived only in
#346, which merged into #333's branch rather than main -- and #333 is
superseded by #353, which does not carry the sunset. Closing #333 without this
would leave hockey the one scoreboard of eight still shipping a fallback.
Same change as the other seven: scroll_display_legacy.py deleted (703 lines),
the guarded import collapsed to a plain one, floor raised to 3.2.0.
test_core_fallback.py -> test_core_scroll.py, identical to afl's but for the
Run: path.
Bumped to 1.22.0 rather than 1.21.0 so it clears every version hockey currently
holds anywhere: 1.20.3 on main, 1.21.0 on #353's branch. A floor is only
meaningful on a version that can actually supersede what users have.
SUNSET_PLUGINS now names all eight, which is the point of listing it rather
than inferring it -- the set is a statement that the sunset holds, and it is
now true of the whole fleet.
Also brings docs/plugin-development/08-shared-sports-code.md up to date. Its
sunset rule still read "Until condition 3 holds, keep the guarded
try-core/except-local import", which was correct in August and is now the
opposite of what the fleet does. Condition 3 holds for src.common.sports_scroll:
the store refuses on all three routes in -- install_plugin (core #431/#433),
the git-pull branch of update_plugin (#508) and install_from_url (#510). The
rule now says to keep the guard for modules that have NOT been through a
sunset, to drop it along with the copy for those that have, and to raise the
floor in the same commit as the deletion. The instruction to keep it in step
with the core doc "in the same PR" is corrected too: they are in different
repositories, so that was never possible.
Verified: all four separator-icon constants and every method survive the
de-indent byte-for-byte; hockey 19 passed, 0 failed; 16 safety-harness renders
pass; five repo gates pass, with check_scroll_adoption now reporting 8 sunset
plugins free of a fallback; fleet 225 passed, 2 skipped, 0 failed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9
---------
Co-authored-by: Claude <noreply@anthropic.com>
* fix(tests): drop a dead import, and name the missing core where it is missed
Two review findings, both valid.
**`import types` was unused** in every copy of test_core_scroll.py. Inherited
rather than introduced: the original test_core_fallback.py never used it
either, and football's copy already on main carries it too. Removed from all
eight, football's included, so the eight stay byte-identical bar the `Run:`
path -- that identity is the property that makes them one rewrite replicated
rather than eight files to keep in step. It was the only F401 in the set.
**test_live_screens.py stubbed `src` even when it could not find a core.** The
stub only receives a `__path__` when discovery succeeds; without one it shadows
the real package, and scroll_display's now-unguarded import fails with
"'src' is not a package" -- naming `src` rather than the core module, which is
exactly the misleading symptom the comment three lines above warns about. It
now says so at the discovery point instead.
Skips rather than fails, exit 2 per run_plugin_tests.py's convention: no core
on the path is a "cannot run here", not a broken plugin. Verified both ways --
with a core the file passes as before, without one it exits 2 and the message
names the real cause and the fix.
Fleet 246 passed, 2 skipped, 0 failed; four gates pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9
---------
Co-authored-by: Claude <noreply@anthropic.com>
The third B6 sunset, after hockey (#346) and football (#349). Stacked on #349 — retarget to
mainonce that lands.Same shape:
scroll_display_legacy.pydeleted, guarded import collapsed to a plain one, manifest floored at 3.2.0. 764 lines removed, −686 net.Baseball is the one with real orchestration logic in the fallback
This is why it was worth doing deliberately rather than by deletion. Baseball's frozen copy carried an extra branch in
_configure_scroll_helperthat no other lineage has: whenscroll_speed × scroll_delayfell outside the 0.1–5.0 pixels-per-frame window butscroll_speedalone sat inside it, it reinterpretedscroll_speedas pixels-per-frame rather than the pixels-per-second the setting is documented as.Measured by running both implementations against the same settings:
I checked all eight lineages: baseball's was the only copy with it.
The core's behaviour is the one to keep. The branch silently ignored the unit the setting is defined in and ran an order of magnitude faster than asked. The core honours the configured pixels-per-second and clamps to the same window — which is what every other scoreboard already does. That is the standardisation.
Re-measured after the collapse, baseball now matches the core exactly at every point in that range:
In practice it reaches nobody. The branch only ever ran on a pre-3.2.0 core; everyone on 3.2.0 or newer has been on the core path since 1.22.0, and the new floor stops this version reaching the rest. It is in the manifest notes anyway — a silently retired behaviour is worse than a documented one.
Verification
MLB_SEPARATOR_ICON,MILB_SEPARATOR_ICON,NCAA_BASEBALL_SEPARATOR_ICON,SCROLL_LEAGUE_KEYS,_SCHEMA_CARD_WIDTH). Review withgit diff -w.test_core_fallback.py→test_core_scroll.py, the same rewrite hockey and football got.SUNSET_PLUGINSgrows to three.Where the sunset stands
Done: hockey (#346), football (#349), baseball (this). Remaining five: afl, basketball, lacrosse, nrl, soccer — all verified to have no orchestration logic in their fallbacks, so those are pure deletions.
🤖 Generated with Claude Code
https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9