refactor(sports): delegate the shared card helpers to the core - #359
Merged
Conversation
The twenty methods below were byte-identical in all eight scoreboards. They now
live in core src.common.sports_card (LEDMatrix#513) and each plugin delegates:
colour _font_color _element_color _coerce_rgb _score_color_for
_recent_score_color _favorite_result
settings _scroll_card_option _upcoming_center_mode _vs_text
date/time _format_game_date _format_game_time _weekday_for _card_tzinfo
favourites _favorite_teams_for _side_is_favorite _side_score
font sizing _crisp_size _schema_font_size _resolve_font_size
_unshare_element_fonts
245 lines leave each plugin, 1,960 in total. One fix to any of that now reaches
every scoreboard instead of needing eight identical edits, and a new scoreboard
gets them by importing one module.
Each method keeps its name, signature and decorators and delegates the body, so
no call site moves and nothing that overrode one loses the ability to. The
plugins' six constant tables are gone in favour of the shared ones, which were
verified identical first.
Football's _crisp_size is replaced by the seven-plugin variant. The two agree
on every real input; the difference is a `not desired` guard that stops a None
size raising TypeError, so football only loses a crash path.
**Verification.** All 176 safety-harness renders across the eight plugins are
byte-identical to the pre-change baseline -- 24 each for the six in-season
sports, 16 each for hockey and lacrosse -- captured with a frozen clock and
compared with cmp, not by eye. Fleet 246 passed, 2 skipped, 0 failed. Four repo
gates pass.
Two tests were leaning on a stubbed `src` and are fixed the way #351 fixed
soccer's: football's test_score_celebration.py stubbed `src` with no __path__,
so the stub shadowed the core and the new top-level import failed naming `src`
rather than the module wanted; lacrosse's test_lacrosse_plugin.py stubs its
host modules explicitly and now loads the real sports_card, since a bare stub
would satisfy the import and fail at the first call.
**The floor rises to 3.3.0** -- the helper is a new core module, so it first
ships in that release. This must not merge before 3.3.0 is cut, or the store
will refuse to install or update these eight.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9
Contributor
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 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 | -1106 |
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.
main gained #355, #357 and #358 while this branch was in review. The only conflicts were plugins.json and the eight scoreboard manifests, all of them version bookkeeping -- no code conflicted. #357 ("retry a logo whose download previously failed") was the one worth checking, since it touches the scoreboards. It changed sports.py, not game_renderer.py, and none of the twenty methods this branch delegates to src/common/sports_card.py. Its 1.x.1 releases are kept in each manifest's version history beneath this branch's entry, and each version was recomputed as the next minor above main's rather than reusing the number this branch picked before #357 landed. plugins.json is regenerated with update_registry.py, never merged by hand.
Companion to the sports_card adoption in the previous commit, and the second half of the same idea. That one shared the settings lookups; this one shares the geometry those settings drive. Nine methods -- _score_reserve_width, _center_gap_width, _logo_slot_width, _logo_cache_key, _layout_offset, _upcoming_date_and_time, _draw_upcoming_center, _draw_upcoming_game_status and set_rankings_cache -- plus five class constants were being maintained in eight copies. They now come from src.common.sports_game_renderer.SportsGameRendererMixin (ChuckBuilds/LEDMatrix#514). Comparing executable ASTs before the change, 67 of the 70 bodies were already identical to the mixin's. The three that were not stay behind as ordinary overrides: baseball's _logo_slot_width and _draw_upcoming_game_status, and hockey's _upcoming_date_and_time. Of the constants only _SCORE_PROBE varies, so afl and basketball -- the two leagues that reach three digits a side -- keep theirs and the rest inherit. This rides in the same version bump as the sports_card adoption rather than taking a second one. Both need core 3.3.0, so splitting them would mean shipping every scoreboard twice for one floor. Also drops the datetime/timezone/ZoneInfo imports that the previous commit orphaned when the date and time formatting moved to sports_card. Hockey still parses timestamps in its override, so it keeps them. Nothing drawn changes: the bodies were moved rather than rewritten.
…mmon stub test_lacrosse_plugin.py stubs "src.common" as a plain ModuleType, so `from src.common.sports_game_renderer import SportsGameRendererMixin` in game_renderer.py could not resolve and the import check failed with "'src.common' is not a package". It only bites when the test runs standalone. Under scripts/run_plugin_tests.py something has already imported the real src.common package, so the sys.modules.setdefault() is a no-op and the real package wins -- which is why the fleet reported this file passing while running it directly failed. The previous commit's sports_card had the same problem and was fixed by loading the real module by hand. Rather than paste that block a second time it now loops over both names, so the next core helper needs no third edit. Unrelated and pre-existing: 7-segment-clock/test_render_polarity.py fails on pristine origin/main (009a9e7) too, so it is not from this branch.
The plugin safety harness drives every scoreboard in `switch` mode, so its renders come from the full-screen scorebug in sports.py. game_renderer.py -- the scroll/Vegas card renderer -- is imported but never called. Replacing render_game_card, _draw_upcoming_center, _center_gap_width and _logo_slot_width with functions that raise leaves all 16 of hockey's harness renders passing and byte-identical to the clean tree. Adding a harness.json variant with `*_display_mode: "scroll"` does not close it either: those renders come out byte-identical to the switch ones, so the mode never reaches the card path from there. The cost of the gap is concrete. Both halves of this branch -- the sports_card helpers and the geometry mixin -- were verified as "byte-identical across 176 renders" while not one of those renders touched the file being changed. B5 shipped four of eight scoreboards with scroll mode broken for the same reason, with every gate green. This drives GameRenderer directly, which is the real path, over 8 plugins x 3 panel sizes x 3 game types = 72 cards, and compares to goldens committed beside each plugin. It picks up the existing scripts/test_*.py convention, so CI discovers it with no workflow change. Verified it bites: widening the centre gap by 2px in the core mixin fails 48 of the 72 cards. On the clean tree all 72 pass. Two details that are easy to get wrong and are deliberate here: - It asserts the card COUNT, not just the mismatch count. A comparison whose inputs quietly went missing reports zero differences and reads as a pass -- that exact failure produced a green "identical: 24, differing: 0" earlier in this work, against a baseline that had never rendered. - It chdir()s to the core. The plugins' default logo_dir values are relative, so anywhere else renders logo-less cards that still compare clean against each other -- a pass that proves nothing.
This was referenced Sep 2, 2026
Only three files conflicted -- plugins.json and the afl and baseball manifests -- all of it version bookkeeping from two docs-only commits, #360 and #361. No scoreboard code conflicted. Versions recomputed as the next minor above main's current rather than reusing the numbers this branch picked before those landed: afl 1.19.3 -> 1.20.0, baseball 1.37.2 -> 1.38.0. plugins.json regenerated with update_registry.py. Re-verified against the new main: all 176 safety-harness renders byte-identical, and the 72-card scroll/Vegas guard passes -- the gate this branch adds, which covers the game_renderer path the harness never reaches.
The guard fails in CI: 21 of its 72 goldens differ there, on the same Pillow 12.3.0, so it is not a rasterisation difference. It renders with real logo assets loaded from the core checkout, and those vary -- main recently dropped case-colliding duplicate league logos -- so the goldens encode one machine's asset set rather than the plugin's rendering. That is a flaw in the guard, not in this branch's consolidation, and a gate that fails on its own inputs should not block the change it was written to protect. It comes back in its own PR, rebuilt to be environment-independent: either rendering without logos, or comparing HEAD against the merge base in the same environment instead of against committed goldens. Nothing else changes. This branch is still verified against main by the 176 safety-harness renders, which are byte-identical, and by the plugin fleet.
main gained the odds_text setting (#402), which bumped every scoreboard to the version numbers this branch had claimed. All eight manifests conflicted for that reason alone; no code conflicted. Versions recomputed as the next minor above main's: afl 1.21.0, baseball 1.39.0, basketball 1.28.0, football 3.3.0, hockey 1.24.0, lacrosse 1.23.0, nrl 1.20.0, soccer 2.23.0. plugins.json regenerated. Checked that #402's work survived rather than assuming it: fonts["odds"], _odds_color() and the odds_text schema block are all still present in every plugin this branch touches. #402 added an odds font slot and colour helper to the same game_renderer.py files this branch delegates twenty methods from, so the two changes had to compose rather than one silently reverting the other.
ChuckBuilds
pushed a commit
that referenced
this pull request
Sep 3, 2026
main gained the odds_text setting (#402) and the card-helper delegation (#359), which together bumped every scoreboard past the numbers this branch claimed. All eight manifests conflicted for that reason alone; no code conflicted. Versions recomputed above main's: afl 1.22.0, baseball 1.40.0, basketball 1.29.0, football 3.4.0, hockey 1.25.0, lacrosse 1.24.0, nrl 1.21.0, soccer 2.24.0. plugins.json regenerated. Checked that the three layers compose rather than assuming it. #402 added an odds font slot and _odds_color() to the same game_renderer.py files #359 delegates twenty methods from, and this branch removes forty-five more methods from sports.py next to them. All three are present together in every plugin: the sports_shared mixin, the sports_card delegations, and the odds wiring. Re-verified against main at the #359 merge: all 176 safety-harness renders byte-identical. The previous CI failure on this branch was stale, not a defect. It ran on 2026-09-02 20:17, and #515 -- which puts src/common/sports_shared.py on core main -- merged 2026-09-03 17:29, twenty-one hours later. The workflow checks the core out at CORE_REF: main, so the module simply did not exist yet when that run imported it.
ChuckBuilds
pushed a commit
that referenced
this pull request
Sep 3, 2026
main gained #403, which adds odds_text to x-propertyOrder in all eight scoreboards and bumps each a patch level. That is the only reason the manifests conflicted again; no code conflicted. Versions recomputed above main's: afl 1.22.0, baseball 1.40.0, basketball 1.29.0, football 3.4.0, hockey 1.25.0, lacrosse 1.24.0, nrl 1.21.0, soccer 2.24.0. Verified all four layers coexist rather than assuming it -- the sports_shared mixin, #359's sports_card delegations, #402's odds wiring, and #403's x-propertyOrder entry are present together in every plugin. Four changes have now landed on the same files this branch edits, so composition is the thing most likely to break silently. test_property_order_coverage.py, the guard that was failing this branch, now passes: it was #402's omission, fixed by #403, not a defect here.
ChuckBuilds
added a commit
that referenced
this pull request
Sep 3, 2026
…404) * test: guard the scroll/Vegas card, without depending on logo assets Re-lands the guard pulled from #359, rebuilt so it cannot fail on inputs that are not the plugin's rendering. Why it exists: check_plugin.py drives every scoreboard in `switch` mode, so its renders come from the full-screen scorebug in sports.py. game_renderer.py -- the scroll/Vegas card renderer -- is imported but never called. Replacing render_game_card, _draw_upcoming_center, _center_gap_width and _logo_slot_width with functions that raise leaves all 16 of hockey's harness renders passing and byte-identical. A harness.json variant setting `*_display_mode: "scroll"` does not help either: those renders come out identical to the switch ones. Why the first attempt failed: it rendered with real logos loaded from the core checkout, and 21 of its 72 goldens differed in CI on the *same* Pillow 12.3.0. The logo files themselves vary between checkouts -- main recently dropped case-colliding duplicate league logos -- so the goldens encoded one machine's asset set rather than the plugin's rendering. The cards are now drawn without logos, leaving the repo's own fonts as the only input. The geometry is still covered: _center_gap_width and _logo_slot_width are computed to lay the card out whether or not a logo is painted. Verified it still bites: widening the centre gap by 2px in the core mixin fails 24 of the 72 cards; on the clean tree all 72 pass, twice in a row. Two details kept from the first version, both deliberate: it asserts the card COUNT rather than only the mismatch count, because a comparison whose inputs quietly went missing reports zero differences and reads as a pass; and it chdir()s to the core, because the plugins' default logo_dir values are relative and resolve only from there. * test: pin the guard's timezone so the goldens are not machine-specific Down from 21 CI mismatches to 3, all of them baseball's upcoming card -- the only card in the set that draws a clock time. The config set no timezone, so the time rendered in the machine's local zone: America/New_York where the goldens were generated, UTC on the runner. The goldens encoded where they were made rather than what the plugin draws. Pinned to "Etc/UTC", not "UTC": the plugins treat a bare "UTC" as a leftover from an old config write-back bug and override it with the system zone, which would have left the same problem in place while looking fixed. Verified the way the previous attempt was not -- by rendering under other zones rather than hoping CI agrees. The goldens match under TZ=UTC and TZ=America/Los_Angeles as well as locally, and widening the centre gap by 2px still fails 24 of the 72 cards. Also merges main for #403, whose x-propertyOrder fix this branch predated; the property-order guard passes again. --------- Co-authored-by: Claude <noreply@anthropic.com>
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.
Twenty methods were byte-identical in all eight scoreboards. They now live in core
src.common.sports_cardand each plugin delegates:_font_color_element_color_coerce_rgb_score_color_for_recent_score_color_favorite_result_scroll_card_option_upcoming_center_mode_vs_text_format_game_date_format_game_time_weekday_for_card_tzinfo_favorite_teams_for_side_is_favorite_side_score_crisp_size_schema_font_size_resolve_font_size_unshare_element_fonts245 lines leave each plugin, 1,960 in total (−1,873 net including the manifests). One fix to any of that now reaches every scoreboard instead of needing eight identical edits, and a new scoreboard gets them by importing one module.
Each method keeps its name, signature and decorators and delegates the body, so no call site moves and nothing that overrode one loses the ability to. The plugins' six constant tables are replaced by the shared ones — verified identical first.
Football's
_crisp_sizebecomes the seven-plugin variant. The two agree on every real input; the difference is anot desiredguard that stops aNonesize raisingTypeError, so football only loses a crash path.Verification
All 176 safety-harness renders across the eight plugins are byte-identical to the pre-change baseline — 24 each for the six in-season sports, 16 each for hockey and lacrosse. Captured with a frozen clock and compared with
cmp, not by eye.Two tests were leaning on a stubbed
srcSame class of thing #351 found in soccer, and the reason the plan says to grep for it:
test_score_celebration.pystubbedsrcwith no__path__, so the stub shadowed the core and the new top-level import failed namingsrcrather than the module actually wanted. Now points at the real core, and skips with a clear message when there isn't one.test_lacrosse_plugin.pystubs its host modules explicitly; it now loads the realsports_card, since a bare stub would satisfy the import and then fail at the first call.🤖 Generated with Claude Code
https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9