fix(scoreboards): retry a logo whose download previously failed - #357
Merged
Conversation
Companion to ChuckBuilds/LEDMatrix#512, which is the other half of the same bug. The core caches a failed logo download as a placeholder wearing the real logo's filename. Fixing the downloader alone does not help these plugins, because they never reach it: the loader scans filename variations, finds the stub, and returns it. The downloader is only consulted when *nothing* is found. So the load path itself has to know. A file that is a placeholder stale enough to be worth retrying no longer counts as a hit in the variations scan, and the download gate drops its `not logo_path.exists()` clause -- that clause is what suppressed the download even after the scan had rejected the stub, since the stub sits at exactly logo_path. Verified end to end against real stubs rather than reasoned about. Five of the eleven cached AFL logos here were 384-byte placeholders written in one bad minute; rendering the AFL scoreboard with the core fix in place took COLL.png from 384 bytes to a 38,612-byte 500x500 logo, and the plugin drew Collingwood's mark instead of a grey box for the first time. `_logo_needs_refresh` imports from the core by full path, never as a bare name: six plugins vendor their own logo_downloader.py, and a deferred bare-name import can bind another plugin's copy once the core isolates top-level plugin modules. The first draft did have the bare-name fallback, and check_module_collisions.py caught it. It is also ImportError-guarded, so against a core predating placeholder marking the check is skipped and behaviour is exactly as before. The helper is copied byte-identically into all ten logo loaders, since the sports engine is duplicated per lineage rather than shared. scripts/test_logo_placeholder_refresh.py holds them in step: it fails if a copy diverges, if a loader is missing the check, if the old existence-trusting gate comes back, or if the bare-name import returns. It also pins the behaviour -- real logos untouched, stale placeholders retried, fresh ones left alone so this does not trade a permanent grey box for a request every frame, and an older or throwing core degrading to the previous behaviour rather than breaking loading. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 | 37 |
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.
Codacy flagged the exec() three ways (use-of-exec, plus two command-injection rules for calling it with a non-literal). The intent was to exercise the helper without importing the whole sports.py, which drags in the core -- but writing the extracted source to a temp file and importing it through importlib does that just as well, with no exec() builtin in sight. It is also better as a test: the helper is now a real module with a real filename, so a traceback points somewhere and coverage can see it. The temp directory is cleaned up via addCleanup. Behaviour is unchanged; all 10 tests still pass, and pyflakes and bandit are clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ChuckBuilds
added a commit
that referenced
this pull request
Sep 2, 2026
#357 took the plugin to 1.19.1 while this was open, so the docs bump moves to 1.19.2 and sits on top of it. Images re-verified byte-identical after the rebase; plugins.json regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ChuckBuilds
added a commit
that referenced
this pull request
Sep 2, 2026
* docs(afl-scoreboard): document every setting, with real finals-series renders Re-lands #356, which merged but never reached main: I based it on docs/7-segment-clock-readme so it could use the render tooling before that tooling existed on main, and #355 squash-merged that branch to main *before* at since. This is the same content, cut from main instead. Documentation only; no behaviour change. The README covered roughly fifteen of the plugin's 120-odd settings, had no images, and its "Key settings" table had drifted from the schema -- it listed show_favorite_teams_only as defaulting to false (it is true), display_duration as 30 (it is 15), and showed show_odds: false in an example labelled as the defaults. Game selection gets its own section, because it is the part that surprises people. There are three distinct code paths -- no favourites, favourites exclusively, and favourites-first-then-others -- and which one runs depends on whether favorite_teams is empty and whether show_favorite_teams_only is on. Most importantly, upcoming_games_to_show and recent_games_to_show mean a per-team budget in the exclusive path and a total in the other two, so three favourites and a value of 3 is nine cards or three depending on one unrelated checkbox. Four dead ends are recorded, each verified rather than assumed: show_odds is a no-op for AFL because ESPN publishes no odds block for the league (a full finals-week payload contains zero) though it still issues one odds request per selected game; show_ranking has no poll to read; and dynamic_duration.min_duration_seconds and background_service.max_workers are in the schema but never applied. Re-verified against main rather than assumed still-current: every documented default still matches config_schema.json after #353 and #354, and the committed images re-render byte-identical against main's sports.py, which those PRs changed. The harness passes 24/24. Version bumped from main's current 1.19.0 rather than the 1.17.3 the stranded branch carried. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore(afl-scoreboard): re-bump to 1.19.2 after the logo fix landed #357 took the plugin to 1.19.1 while this was open, so the docs bump moves to 1.19.2 and sits on top of it. Images re-verified byte-identical after the rebase; plugins.json regenerated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
ChuckBuilds
pushed a commit
that referenced
this pull request
Sep 2, 2026
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.
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
1 participant
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.
Companion to ChuckBuilds/LEDMatrix#512 — the two halves of one bug. That PR can merge independently; this one is inert without it (the check is
ImportError-guarded).Why the core fix alone isn't enough
The core caches a failed logo download as a placeholder wearing the real logo's filename. Fixing the downloader doesn't help these plugins, because they never reach it:
The stub is found by the scan, so the downloader is never consulted.
The change
Two lines per loader:
not logo_path.exists(). That clause suppressed the download even after the scan rejected the stub — the stub sits at exactlylogo_path.Plus a copied
_logo_needs_refreshhelper.Verified end to end, not reasoned about
Five of eleven cached AFL logos here were 384-byte placeholders written in one bad minute on Sep 1. With the core fix in place, rendering the AFL scoreboard:
Collingwood's mark drew instead of a grey box for the first time.
Two things I got wrong first, and the guards that caught them
from logo_downloader import .... Six plugins vendor their ownlogo_downloader.py, and a deferred bare-name import can bind another plugin's copy once the core isolates top-level modules.check_module_collisions.pycaught it; the helper now imports by full path only.win_color/loss_coloras dead in the AFL pass — they're read via an f-string key. Unrelated, but it's why everything above is verified against real files rather than asserted.Keeping ten copies in step
The sports engine is duplicated per lineage, so the helper is copied byte-identically into all ten logo loaders (nine
sports.pyplusbaseball-scoreboard/logo_manager.py).scripts/test_logo_placeholder_refresh.pyfails if a copy diverges, a loader loses the check, the existence-trusting gate returns, or the bare-name import comes back.It also pins behaviour: real logos untouched, stale placeholders retried, fresh ones left alone so this doesn't trade a permanent grey box for a request every frame, and an older-or-throwing core degrading to previous behaviour rather than breaking logo loading.
ufc-scoreboardis included despite being third-party-authored — leaving one lineage behind on shared code is worse than the ownership question.Verification
python scripts/test_logo_placeholder_refresh.py— 10/10python scripts/run_plugin_tests.pyacross all 9 touched plugins — 188 passed, 4 failed, all 4 pre-existing onmain(3 needRGBMatrixEmulator, 1 prompts interactively)python scripts/check_plugin.pyon football / baseball / afl — 24/24 PASS eachpython scripts/check_module_collisions.py— OK across 43 pluginsplugins.jsonregenerated🤖 Generated with Claude Code