fix(plugin-harness): add no-op process_deferred_updates to test double - #391
Merged
Merged
Conversation
The safety harness's VisualTestDisplayManager (base of BoundsCheckingDisplayManager) doesn't implement process_deferred_updates(), which 5 first-party ledmatrix-plugins call unconditionally between set_scrolling_state() and their scroll-position update: news, odds-ticker, ledmatrix-leaderboard, stock-news, and ledmatrix-stocks. Any of them fails the harness with AttributeError the moment it's touched (surfaced when ledmatrix-plugins#177 had to add a local hasattr guard in ledmatrix-stocks just to pass CI). Add the method as a no-op, mirroring the existing "no-op for testing" pattern already used for set_scrolling_state, so these plugins render under the harness without every touching PR needing its own guard.
Contributor
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesVisual display deferred updates
Estimated code review effort: 1 (Trivial) | ~5 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 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 |
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 reviewerTIP This summary will be updated as you push new changes.
Uh oh!
There was an error while loading. Please reload this page.
ChuckBuilds added a commit
that referenced
this pull request
Sep 8, 2026
… size (#539) * fix(display): pin one text layout engine, and give the 5x7 face a size Two ways a font could render differently on two machines running the same code, both found while diagnosing four plugins whose golden images passed on the machine that generated them and failed everywhere else. **Layout engine.** `ImageFont.truetype` picks its engine at load time: Raqm where the host Pillow was built with libraqm, Basic otherwise. The two round fractional glyph advances differently. `PressStart2P-Regular.ttf` at 8px has whole-pixel advances, so they agree — which is why most of the fleet matched everywhere and hid this. `4x6-font.ttf` at 6px does not: glyph positions drift cumulatively along a run, and the four plugins that draw body text in it (geochron, of-the-day, christmas-countdown, ledmatrix-weather's almanac) are exactly the four whose goldens travelled badly. Every core font load now goes through `src/common/font_layout.load_truetype`, which pins the Basic engine, so a render depends on the font file and the size and nothing else. Basic gives up complex-script shaping and kerning pairs; neither applies to bitmap-grid faces on an LED panel. Output is unchanged on a host without libraqm. **Zero font height.** `DisplayManager` built the 5x7 BDF face with `freetype.Face(path)` and never called `set_char_size`, so `face.size.height` stayed 0 and `get_font_height()` returned 0 for it — callers stacking rows by `prev_y + prev_height + gap` drew two lines on top of each other. The start-up line `Calendar font size: 0 pixels` has been printing the symptom all along. `font_manager._load_bdf_font` already called `set_char_size`, so whether measurement worked depended on which path loaded the face. `DisplayManager` now sets it too, and `get_font_height()` falls back to the strike the file declares rather than returning a zero line height. FixesChuckBuilds/ledmatrix-plugins#397 Refs ChuckBuilds/ledmatrix-plugins#371, #375, #378, #391 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(display): give the startup banner a rung that fits a full address at 64px CI caught what pinning the layout engine exposed rather than caused. `_fitting_font` walks PressStart2P then 4x6 at 6px, and "255.255.255.255" -- the widest thing the startup banner ever shows -- measures 66px at 4x6/6px against the 62 a 64x32 panel has to give. It used to squeak in only because the measurement depended on which layout engine the host Pillow happened to have; with the engine pinned it does not, so the rung the worst case actually needs is now in the ladder instead of implied: 4x6 at 5px, which measures 51. The fallback was wrong in the same place. When nothing in the ladder fit, it returned `self.font` -- the *widest* option, and precisely how "Initializing" came to run off the side of a 64px panel to begin with. It returns the narrowest face that loaded now. test/test_initializing_screen.py: 34 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(display): name the exceptions the BDF strike read can raise Codacy flagged the try/except/pass. It was already narrow in intent -- a malformed strike table on the measurement path must degrade to "size unknown" rather than take the display down -- but a bare `except Exception: pass` says neither of those things and hides a genuinely broken font behind a silent 8px fallback. It now catches what reading `available_sizes` can actually raise and logs which face failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore: drop logo PNGs the render harness downloaded into the worktree These are fetched at runtime by the logo cache; they are not source, and they rode in on a `git add -A` while I was running check_plugin.py against this branch. Nothing in the change needs them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <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 freeto 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.
Summary
The plugin-safety harness's
VisualTestDisplayManager(base class ofBoundsCheckingDisplayManager) doesn't implementprocess_deferred_updates().Five first-party plugins in
ledmatrix-pluginscall it unconditionally,right alongside
set_scrolling_state(), before updating scroll position:news,odds-ticker,ledmatrix-leaderboard,stock-news, andledmatrix-stocks. Any of them fails the harness withAttributeErrorthe instant it's touched, even though the call is completely safe against
the real production
display_manager.py(which has always had thismethod).
This surfaced concretely in ledmatrix-plugins#177,
which had to add a local
hasattr(self.display_manager, "process_deferred_updates")guard in
ledmatrix-stocks/manager.pyjust to get CI green — a workaroundfor this exact gap, not a real plugin bug. The other 4 plugins are still
unguarded and will hit the same failure the next time any of them is
touched.
Fix
Add
process_deferred_updates()toVisualTestDisplayManageras a no-op,mirroring the existing "no-op for testing" pattern already used a few lines
above for
set_scrolling_state(). This double has no deferred-update queueto process, so there's nothing to do — the point is just to stop raising
AttributeErrorfor callers that assume the method exists (as productioncode always can).
Test plan
test_process_deferred_updates_is_nooptotest/plugins/test_visual_rendering.py,next to the existing
test_scrolling_statetest, asserting the calldoesn't raise and doesn't disturb scrolling state.
python3 -m py_compileon both touched files.test/plugins/test_harness.py,test_visual_rendering.py,and
test_plugin_matrix.pyto confirm none of them do exhaustivemethod-set introspection that this addition would break.
test.yml→pytest test/plugins/test_harness.py test/plugins/test_visual_rendering.py test/plugins/test_plugin_matrix.py)will exercise this on the PR.
Follow-up (not in this PR)
Once this merges, the local
hasattrguard added inledmatrix-plugins#177becomes unnecessary (though harmless to leave). Theother 4 affected plugins (news, odds-ticker, leaderboard, stock-news) need
no changes at all — they'll simply stop failing the harness next time
they're touched.
Summary by CodeRabbit
Bug Fixes
Tests