fix(scoreboards): render the font each element's schema advertises - #409
Merged
Conversation
Nine elements advertised one font in the settings page and drew another. The status text on seven plugins, plus hockey's detail and odds text, declared 4x6-font.ttf in config_schema.json while rendering PressStart2P-Regular.ttf. The call sites never passed default_font, so _load_custom_font fell back to its own default face instead of the schema's. Picking the font the picker already showed as active therefore changed the display, which reads as a broken picker. It is also why hockey's odds looked unlike every other sport. Pre-existing, not from the consolidation: the missing argument is identical before and after it. The fix is schema-driven per plugin, not a blanket change. Baseball's schema genuinely declares PressStart2P for status, so baseball is untouched. Football needed a second edit -- its status default also lives in the style-resolver table _LOADER_DEFAULTS, so patching the call site alone left it mismatched. Hockey's game_renderer._load_custom_font gains the default_font parameter its seven siblings already have. A first attempt at this passed the keyword without checking, which raised TypeError into a bare except and silently sent *every* hockey font to the emergency fallback -- the same swallow-and-continue shape as the bug this branch is fixing. Measured impact: 42 of 176 safety-harness renders, all on panels 128px wide and narrower, all upcoming screens. Wider panels draw that header with the time font (`if display_width > 128`) and are byte-identical. It also fixes a knock-on defect the code comments about: "Next Game" is 72px in PressStart2P@8 and had to shed to "Next" on a 64px panel; at 4x6-font@7 it is 41px and fits whole. football/test_fonts_render_crisply.py needed its discriminator changed, not weakened. It tells "the config path ran" from "the emergency fallback ran", and used the status face to do it -- which stops discriminating once both paths agree on 4x6-font. It now keys on the odds slot, which only the config path builds. Verified it still fails when a bad keyword is injected into that path, which is the bug it exists to catch.
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 | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewerTIP This summary will be updated as you push new changes.
ChuckBuilds pushed a commit
that referenced
this pull request
Sep 4, 2026
Addresses three CodeRabbit findings on #417, all of them real. The first is the one that mattered: ledmatrix-flights had six anti-aliased draws this PR had already claimed to fix. None of its ten Draw sites calls .text() itself -- every renderer hands the Draw to _draw_centered()/_draw(), which do. The first version of this gate matched `<var>.text(` file-wide and caught them; tightening it to same-scope AST matching to cut false positives threw the real findings away and reported the file clean. The runtime probe missed them too, because the harness never renders those flight paths. So the gate now resolves, to a fixpoint, which functions draw text on a parameter, and treats a Draw handed to one of those as text-rendering. That sits between the file-wide regex (130 findings, mostly noise) and same-scope matching (missed real ones): an overlay Draw passed to a compositing helper is still ignored, while _draw_centered(draw, ...) counts. Two smaller gate defects, also reported and also real: * any `.fontmode` assignment satisfied the check, so `fontmode = "L"` -- the anti-aliasing default -- would have passed. Now only the constant "1". * ast.walk() descended into nested scopes and ignored statement order, so a fontmode set *before* its Draw() counted. Now scoped and ordered. Each is mutation-tested: removing a hand-off fontmode, setting it to "L", or moving it above its Draw() each make the gate fail. That found 40 further sites in 13 plugins, including overlay and celebration paths in all eight scoreboards that the harness never renders. Versions are picked above every number claimed by #409 and #412; football takes 3.4.3 so #424 keeps 3.5.0. Merge order: #409, #412, this, then #424. Verified: 246 passed / 2 skipped / 0 failed, 72-card scroll guard passes, all repo gates pass, and the runtime probe still reports 0 anti-aliased text draws across 31 plugins at every panel size. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9
Uh oh!
There was an error while loading. Please reload this page.
ChuckBuilds added a commit
that referenced
this pull request
Sep 4, 2026
The merge from main left the 1.24.2 entry holding both sides bodies -- two notes keys, two changelog keys, and a missing comma between them -- so manifest.json was not valid JSON and the version gate could not read it. Both sides had added a 1.24.2: main from the font fix in #409, this branch from the README rewrite. Rebuilt from main manifest, which is intact, with the docs change moved to a new 1.24.3 entry above it. That keeps main 1.24.2 exactly as merged and follows the shape the other docs PRs used on main -- the new entry carries the previous entry notes and changelog plus its own changes array. plugins.json regenerated: latest_version 1.24.3. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ChuckBuilds added a commit
that referenced
this pull request
Sep 4, 2026
…rs (#428) * docs(lacrosse-scoreboard): document all 170 settings, with real renders The README documented a fraction of the plugin's 170 settings and had no images. It now covers every schema leaf -- verified by a token audit -- and shows the three men's display modes, both leagues side by side, the show_records toggle, and the card at four panel sizes. The two league blocks are identical in every setting except one: ncaa_mens defaults to enabled and ncaa_womens does not. Documenting one <league> table rather than two identical sets keeps 104 of the 170 leaves readable instead of duplicated. Two things the old README did not say: - display_options.show_records and show_ranking default to true per league, while defaults.show_records and defaults.show_ranking default to false, and the per-league copy wins. Because the UI writes schema defaults on save, changing the defaults copy appears to do nothing. Both the settings reference and troubleshooting now say so. - The favorites key here is teams.favorite_teams_only, not show_favorite_teams_only as in the single-league scoreboards, and it defaults to off rather than on. other_games_min_quality is live in this plugin -- NCAA lacrosse has a national poll to rank against -- unlike in nrl, where it is inert. other_games_divisions stays inert; it is a college football taxonomy. Renders come from docs/assets/lacrosse-scoreboard/shots.json. Lacrosse keeps one manager per league and mode as a plain attribute rather than in a dict, and only builds them when that league's enabled flag is set, so the fixture turns both leagues on. Real ESPN crests, not placeholders. check_plugin.py passes with no FAIL. Carries the docs-tooling changes from #423, which this render depends on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs(lacrosse-scoreboard): keep the lax_ prefix upgrade notice The rewrite dropped the breaking-change notice from 1.1.0, when the display modes gained a lax_ prefix to stop colliding with the NCAA hockey modes. Anyone still holding an old pin in display_durations or rotation_order needs it, and the CHANGELOG mapping is the only other place it is written down. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(lacrosse-scoreboard): repair the manifest after the merge The merge from main left the 1.24.2 entry holding both sides bodies -- two notes keys, two changelog keys, and a missing comma between them -- so manifest.json was not valid JSON and the version gate could not read it. Both sides had added a 1.24.2: main from the font fix in #409, this branch from the README rewrite. Rebuilt from main manifest, which is intact, with the docs change moved to a new 1.24.3 entry above it. That keeps main 1.24.2 exactly as merged and follows the shape the other docs PRs used on main -- the new entry carries the previous entry notes and changelog plus its own changes array. plugins.json regenerated: latest_version 1.24.3. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
ChuckBuilds added a commit
that referenced
this pull request
Sep 4, 2026
scripts/test_scroll_card_renders.py has been red on main since #409, which made the safety check fail on every open PR regardless of what that PR touched -- #428 fails it while changing only one plugin's README and manifest. The signal was useless: a real render regression would have looked identical to this noise. #409 deliberately changed which font several text elements draw in, so the goldens committed on 2026-08-02 describe the old, wrong rendering. Measured: at the parent of #409 the guard reports 3 problems, on main it reports 42. These 39 are that difference. Checked rather than assumed. Every regenerated card was compared against its predecessor pixel by pixel, and the four sampled visually: the text content is identical and only the glyph shapes change, which is what a font swap should look like. --update also rewrote 24 cards whose pixels had not changed at all, presumably encoder metadata; those are reverted so the diff is only the 39 that actually differ. Baseball's three upcoming cards are deliberately NOT refreshed. They are the 3 that were already failing before #409, and they fail for an unrelated reason: game_renderer.py formats the date with %-m/%-d and %-I:%M%p, which are glibc extensions. Off glibc they raise, the except falls back to a raw ISO string, and the card loses its start time and shows 2026-09-19 instead of Sep 19. The committed goldens are correct; regenerating them anywhere but glibc would bake the broken output in. Filed separately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 4, 2026
ChuckBuilds added a commit
that referenced
this pull request
Sep 5, 2026
scripts/test_scroll_card_renders.py has been red on main since #409, which made the safety check fail on every open PR regardless of what that PR touched -- #428 fails it while changing only one plugin's README and manifest. The signal was useless: a real render regression would have looked identical to this noise. #409 deliberately changed which font several text elements draw in, so the goldens committed on 2026-08-02 describe the old, wrong rendering. Measured: at the parent of #409 the guard reports 3 problems, on main it reports 42. These 39 are that difference. Checked rather than assumed. Every regenerated card was compared against its predecessor pixel by pixel, and the four sampled visually: the text content is identical and only the glyph shapes change, which is what a font swap should look like. --update also rewrote 24 cards whose pixels had not changed at all, presumably encoder metadata; those are reverted so the diff is only the 39 that actually differ. Baseball's three upcoming cards are deliberately NOT refreshed. They are the 3 that were already failing before #409, and they fail for an unrelated reason: game_renderer.py formats the date with %-m/%-d and %-I:%M%p, which are glibc extensions. Off glibc they raise, the except falls back to a raw ISO string, and the card loses its start time and shows 2026-09-19 instead of Sep 19. The committed goldens are correct; regenerating them anywhere but glibc would bake the broken output in. Filed separately. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
ChuckBuilds added a commit
that referenced
this pull request
Sep 5, 2026
…417) * fix(plugins): draw text 1-bit, so glyphs stay crisp on the LED grid An LED panel has no partial brightness. PIL defaults ImageDraw's fontmode to "L", which anti-aliases TrueType glyphs into a grey fringe the panel can only round off -- a 4px glyph arrives smeared into 3px. Measured at draw time rather than grepped, because the source-level signal misleads in both directions. A missing fontmode is often harmless: birdnet-go and youtube-stats draw PressStart2P at its native 8px, which emits no partial pixels whatever the mode. A present one is not coverage: ledmatrix-flights set it at 3 of its 12 Draw() sites, and the one that actually blurred was not among them. pomodoro-timer looked like the worst offender and was already correct -- it masks, thresholds at >=128 and paints flat, so only its intermediate mask was ever anti-aliased. Hooking ImageDraw.text across 31 plugins x 8 panel sizes found 18 genuinely anti-aliased sites in 9 plugins, all of them 4x6-font.ttf at 6 (74% of lit pixels at partial coverage) or PressStart2P off its 8px grid. The other 52 sites here were clean only by luck: their faces happen to sit on the pixel grid at the size currently configured. Font sizes are user settings, so that is not a property to rely on -- typing 10 into the web UI reintroduces the blur. The setting belongs on every draw that renders text. Excluded: the offline asset generators (download_assets.py, logo_downloader.py and friends). They bake placeholder logos on a developer machine, not text on the panel, and logo resampling is a separately-decided question. Not render-verified: mqtt-notifications, nfl-draft, on-air and static-image render only with live data the harness has none of. They are fixed statically, on the same reasoning, but no render proves it. scripts/test_pixel_perfect_text.py keeps it from coming back, and is wired into the Plugin Structure workflow. It is AST-scoped, not a file-wide regex: a Draw() is reported only where that binding is used for .text() in the same function, which is the difference between 43 real findings and 130 mostly spurious ones. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 * fix(plugins): catch draws handed to a text-drawing helper Addresses three CodeRabbit findings on #417, all of them real. The first is the one that mattered: ledmatrix-flights had six anti-aliased draws this PR had already claimed to fix. None of its ten Draw sites calls .text() itself -- every renderer hands the Draw to _draw_centered()/_draw(), which do. The first version of this gate matched `<var>.text(` file-wide and caught them; tightening it to same-scope AST matching to cut false positives threw the real findings away and reported the file clean. The runtime probe missed them too, because the harness never renders those flight paths. So the gate now resolves, to a fixpoint, which functions draw text on a parameter, and treats a Draw handed to one of those as text-rendering. That sits between the file-wide regex (130 findings, mostly noise) and same-scope matching (missed real ones): an overlay Draw passed to a compositing helper is still ignored, while _draw_centered(draw, ...) counts. Two smaller gate defects, also reported and also real: * any `.fontmode` assignment satisfied the check, so `fontmode = "L"` -- the anti-aliasing default -- would have passed. Now only the constant "1". * ast.walk() descended into nested scopes and ignored statement order, so a fontmode set *before* its Draw() counted. Now scoped and ordered. Each is mutation-tested: removing a hand-off fontmode, setting it to "L", or moving it above its Draw() each make the gate fail. That found 40 further sites in 13 plugins, including overlay and celebration paths in all eight scoreboards that the harness never renders. Versions are picked above every number claimed by #409 and #412; football takes 3.4.3 so #424 keeps 3.5.0. Merge order: #409, #412, this, then #424. Verified: 246 passed / 2 skipped / 0 failed, 72-card scroll guard passes, all repo gates pass, and the runtime probe still reports 0 anti-aliased text draws across 31 plugins at every panel size. 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>
ChuckBuilds added a commit
that referenced
this pull request
Sep 7, 2026
…olver (#462) * test(scroll-cards): refresh the goldens #409 made stale scripts/test_scroll_card_renders.py has been red on main since #409, which made the safety check fail on every open PR regardless of what that PR touched -- #428 fails it while changing only one plugin's README and manifest. The signal was useless: a real render regression would have looked identical to this noise. #409 deliberately changed which font several text elements draw in, so the goldens committed on 2026-08-02 describe the old, wrong rendering. Measured: at the parent of #409 the guard reports 3 problems, on main it reports 42. These 39 are that difference. Checked rather than assumed. Every regenerated card was compared against its predecessor pixel by pixel, and the four sampled visually: the text content is identical and only the glyph shapes change, which is what a font swap should look like. --update also rewrote 24 cards whose pixels had not changed at all, presumably encoder metadata; those are reverted so the diff is only the 39 that actually differ. Baseball's three upcoming cards are deliberately NOT refreshed. They are the 3 that were already failing before #409, and they fail for an unrelated reason: game_renderer.py formats the date with %-m/%-d and %-I:%M%p, which are glibc extensions. Off glibc they raise, the except falls back to a raw ISO string, and the card loses its start time and shows 2026-09-19 instead of Sep 19. The committed goldens are correct; regenerating them anywhere but glibc would bake the broken output in. Filed separately. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(scroll): route every scrolling plugin through the shared resolver Eleven plugins each hand-rolled scroll configuration and disagreed with one another about what identical config meant. odds-ticker ranked the deprecated scroll_pixels_per_second ABOVE the documented scroll_speed/scroll_delay pair, and because that key carries a schema default the documented settings were dead for every user (#408). ledmatrix-leaderboard read the same key only as a fallback, so the same config produced different speeds in the two plugins. stock-news derived px/frame through its own arithmetic. Nobody was wrong locally; they were just eleven answers to one question. All of them now call src.common.scroll_config.configure(), which resolves every supported config shape in one place, snaps the speed to one the panel can render in whole pixels, and reports the frame hold needed to keep slow speeds crisp. Identical config now means an identical speed everywhere. Each plugin keeps its original logic as a fallback behind try: from src.common import scroll_config as _scroll_config except ImportError: _scroll_config = None because plugins update independently of the core and must keep working against one that predates the helper. Where the legacy block sits inline in __init__ the shared call runs after it and wins, rather than re-indenting logic other config shapes still depend on; those plugins log a line saying so, because two scroll speeds in the journal with no indication which took effect is exactly the confusion this change exists to remove. Plugins pass settings.frame_hold to set_scrolling_state(True, ...) when they begin scrolling. The hold cannot be applied once at construction: plugins share one display manager, and it is reset whenever any other plugin finishes its scroll. Verified on hardware (Pi 4, 2x128x64, 100Hz). Four enabled plugins resolve through the helper with no tracebacks, and ledmatrix-stocks at 50 px/s measures 50.0 fps, median 20.00ms -- exactly two refreshes per frame, with the panel still refreshing at 100Hz so flicker is unchanged. Requires core support: ChuckBuilds/LEDMatrix#523. Without it every plugin takes the fallback path and behaves exactly as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(odds-ticker): make the documented scroll settings reachable (#408) Two defects meant display_options.scroll_speed / scroll_delay -- the format the plugin documents and recommends -- could never take effect. The priority-1 branch read scroll_pixels_per_second into self rather than clearing it the way the display_config branch does. config_schema.json gives that deprecated key a default of 50.0 and schema defaults are merged into plugin config, so it was never None and use_frame_based() below was never True. Separately, use_frame_based() only ever inspected display_config, so even with that fixed the recommended shape still could not select frame-based mode. Both halves were needed. The visible symptom was the plugin logging its scroll configuration twice on startup, ~0.36s apart, the second line quietly overriding the first: Using display_options.scroll_speed=1.0 px/frame ... (frame-based mode) Using scroll_pixels_per_second: 50.0 px/s (time-based mode) with the second one being what actually ran. On a 100Hz panel that meant 0.5px per frame, so every second frame rendered identically, dirty tracking skipped the panel swap, and the ticker juddered. Editing scroll_speed had no effect, and deleting scroll_pixels_per_second only restored the schema default. Also takes the live-game check off the render path. display() called _get_current_update_interval() every frame, and its slow path reads the scoreboard cache from disk and parses JSON per enabled league -- producing a single ~15ms frame every few minutes, measurable as a stall mid-scroll. The interval is now memoised for 15s; the scoreboard re-check underneath is rate limited to 300s regardless, so live detection is unaffected. The neighbouring debug line used an f-string, so it called _has_live_games() on every skipped update even with debug logging off; it now uses %s args. Verified on hardware: odds-ticker selects frame-based mode with no time-based line following it, and resolves to 100.0 px/s at 1px per refresh. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: make the frame-hold call safe for the test doubles, and update to main Three display-manager doubles still declared set_scrolling_state with a single argument, so the frame_hold this PR passes raised TypeError mid-render. display() swallows that in its own except Exception, leaving only "no frame reached the display" -- odds-ticker's cache-invalidation test failed for a reason that had nothing to do with cache invalidation. CI caught one of the three; the other two never reach the scrolling branch. scripts/test_scroll_state_doubles.py checks the invariant instead, and catches all three (including the *args one, which looks tolerant but cannot take a keyword). odds-ticker's double also needed _scroll_frame_hold itself: it borrows only the methods under test, and display() now calls that one. Fixes a continuation line in odds-ticker/manager.py that was indented back to column 12 inside the call -- legal, but not what anyone meant. Merges main. This branch was two commits behind, which made its diff look like it deleted versions[] entries main had gained. Six plugins then needed version bumps they did not need before, because main had bumped past them. ledmatrix-leaderboard and nfl-draft sit one patch above #466's numbers rather than duplicating them -- two PRs must never publish the same version, and a user could not tell which build they have. That pins the merge order these two already needed. Verified against a core with #523 and #534 merged: safety harness clean on all 11 changed plugins, 23 unit tests pass, version and collision gates pass. The one failing repo guard, test_odds_centre_collision.py, fails identically on main and is untouched here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: t <t@l>
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Nine elements advertised one font in the settings page and drew another.
The call sites never passed
default_font, so_load_custom_fontused its own fallback face instead of the schema's. Picking the font the picker already showed as active therefore changed the display — which reads as a broken picker. It's also why hockey's odds looked unlike every other sport.Pre-existing, not from the consolidation — the missing argument is identical before and after it.
Schema-driven, not a blanket change
Baseball's schema genuinely declares PressStart2P for
status_text, so baseball is untouched. Each plugin now renders what its own schema declares.Football needed a second edit: its status default also lives in the style-resolver table
_LOADER_DEFAULTS, so patching the call site alone left it mismatched.Hockey's
game_renderer._load_custom_fontgains thedefault_fontparameter its seven siblings already have. My first attempt passed the keyword without checking, which raisedTypeErrorinto a bareexceptand silently sent every hockey font to the emergency fallback — the same swallow-and-continue shape as the bug being fixed here. The corrected pass reads each file's actual signature first.Measured impact
42 of 176 safety-harness renders, tightly scoped:
if display_width > 128)upcomingonlyIt also fixes a knock-on defect the code comments about:
"Next Game"is 72px in PressStart2P@8 and had to shed to"Next"on a 64px panel; at 4x6-font@7 it is 41px and fits whole.The one test that changed
football/test_fonts_render_crisply.pydistinguishes "the config path ran" from "the emergency fallback ran" — a real hazard, since_load_fontswraps everything intry/exceptand drops to crisp hardcoded defaults, hiding a broken call.It used the status face as the discriminator, which stops discriminating once both paths agree on 4x6-font. The discriminator was replaced, not the check weakened: it now keys on the
oddsslot, which only the config path builds. Verified it still fails when a bad keyword is injected into that path.Fleet: 245 passed, 1 pre-existing unrelated failure resolved by this branch's test update.