fix(sports): let the plugin declare its own directory, don't deduce it - #519
Merged
Conversation
_plugin_dir() returned None on every device. The consequence was silent and
reached the panel:
_plugin_dir() -> None
_schema_font_size() -> None for every element
-> a configured size equal to the schema default stops looking like a
default and is treated as a deliberate user choice
-> the snap to the font's pixel grid is skipped
-> 4x6-font.ttf renders at 6 instead of 7: 3px-wide glyphs, not 4px
On a 256x64 panel that made the odds, the team records and the date row hard to
read. Both `odds` and `detail` were affected -- anything resolving a
grid-snapped schema default was a pixel narrow.
Why it was invisible here. PluginLoader._namespace_plugin_modules renames every
bare module a plugin brought in (sports, game_renderer, ...) to
"_plg_<plugin_id>_<module>" and REMOVES the bare sys.modules entry, so two
plugins owning a module of the same name cannot collide. A class defined in
sports.py still reports __module__ == "sports", but sys.modules["sports"] is
gone, so walking the MRO for a module with a __file__ finds nothing.
Every test here imported plugins directly, which leaves the bare entry in
place, so the walk succeeded. The safety harness loads plugins its own way and
never reproduced it either. It was found by a user counting pixels on the
panel.
The directory is now declared by the plugin (_PLUGIN_DIR) and only deduced as
a fallback, for hosts that declare nothing -- the plugins' own probe harnesses
build classes with type().
Verified on hardware, which is the only place the original failure appeared:
before, the live service logged plugin_dir=None and 4x6-font.ttf@6 for all six
football managers; after, plugin_dir resolves and both odds and detail are @7.
Five regression tests, including the production shape: a class whose __module__
is absent from sys.modules still resolves via its declared directory, and the
precondition that the MRO walk alone returns None is pinned so the test keeps
meaning something if the fallback changes.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds shared sports mixins and centralizes common scoreboard behavior. It also lets plugins declare their directory through ChangesShared sports plugin infrastructure
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This change enables sports plugins to declare validated directories so schema defaults and font grid snapping resolve correctly, with regression coverage for the production loader scenario. No current merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
ChuckBuilds
added a commit
to ChuckBuilds/ledmatrix-plugins
that referenced
this pull request
Sep 3, 2026
Every grid-snapped font was rendering a pixel narrow on every device. The shared sports code reads this plugin's config_schema.json to tell a default font size from one the user chose: a default is snapped to the font's pixel grid, a choice is left alone. It located the schema by inspecting loaded modules, which cannot work under the real plugin loader -- PluginLoader._namespace_plugin_modules renames a plugin's modules to "_plg_<id>_<module>" and removes the original names, so nothing is left to inspect. The lookup returned nothing, every configured size then looked user-chosen, the snap was skipped, and 4x6-font.ttf drew at 6 instead of 7: 3px-wide glyphs instead of 4px. On a 256x64 panel the betting odds, the team records and the date row were hard to read. Each SportsCore now declares _PLUGIN_DIR from its own __file__, which the shared code prefers over guessing (ChuckBuilds/LEDMatrix#519). scripts/test_plugin_dir_under_loader.py stops this recurring. It reproduces the loader's sequence exactly -- import sports.py under its BARE name, then rename it away -- because that is what makes the class report __module__ == "sports" with no such entry in sys.modules. Both halves matter, and the first draft of this guard got it wrong: it imported under the namespaced name, __module__ still resolved, and it passed against the broken code. Verified the corrected version fails for all eight with the declarations removed and passes with them restored. Found by a user counting pixels on a photo of the panel. No gate here caught it: every test imported plugins directly, which leaves the bare module entry in place, and the safety harness loads plugins its own way. Co-authored-by: Claude <noreply@anthropic.com>
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
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.
ChuckBuilds
added a commit
that referenced
this pull request
Sep 4, 2026
…ilently (#520) _schema_font_size swallowed every exception and cached an empty dict. That is not cosmetic. With no schema, a configured font size can no longer be compared against the schema default, so every size is treated as a deliberate user choice and skips the snap to the font's pixel grid -- which renders 4x6-font.ttf at 6 instead of 7: a 3px-wide glyph instead of 4px. That shipped. On a 256x64 panel it made the odds, the team records and the date row hard to read, and it was found by a user counting pixels on a photo of the panel rather than by anything here. The cause (_plugin_dir returning None under the real plugin loader) is fixed in #519; this makes the same class of failure audible next time: Orphan: could not read config_schema.json (FileNotFoundError: ...); every font size will be treated as user-chosen and will skip its pixel grid snap. Font sizes may render a pixel narrow. The message names the consequence, not just the error, because the error alone does not suggest "your fonts are a pixel narrow". Logged rather than raised: an unreadable schema must not stop a plugin rendering. The cache is built once per class (per schema path in sports_card), so this cannot repeat per frame. Scope deliberately small. An audit of the three shared modules found 23 handlers that swallow and return a default, but all 23 catch specific types -- TypeError, ValueError, ImportError -- turning bad config values into defaults, which is what they are for. Of 77 broad handlers across the font and odds paths, 74 already log. Only these two were both broad and silent.
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.
_plugin_dir()returnedNoneon every device. The consequence was silent and reached the panel:On a 256×64 panel that made the odds, the team records and the date row hard to read. Both
oddsanddetailwere affected — anything resolving a grid-snapped schema default was a pixel narrow.Why no gate caught it
A class defined in
sports.pystill reports__module__ == "sports", butsys.modules["sports"]no longer exists — so walking the MRO for a module with a__file__finds nothing.Every test here imported plugins directly, which leaves the bare entry in place, so the walk succeeded. The safety harness loads plugins its own way and never reproduced it either. It was found by a user counting pixels on a photo of the panel.
Fix
The directory is declared by the plugin (
_PLUGIN_DIR), and only deduced as a fallback for hosts that declare nothing — the plugins' own probe harnesses build classes withtype().Needs the companion plugins PR, which adds the one-line declaration to each of the eight scoreboards.
Verified on hardware
The only place the original failure appeared:
plugin_dir=None,4x6-font.ttf@6for all six football managersplugin_dir=/…/football-scoreboard,odds=@7 detail=@7Five regression tests, including the production shape: a class whose
__module__is absent fromsys.modulesstill resolves via its declared directory. The precondition — that the MRO walk alone returnsNone— is pinned, so the test keeps meaning something if the fallback ever changes.Summary by CodeRabbit
Bug Fixes
Refactor