feat: adaptive layout system — size-aware regions, crisp font ladders, image fitting#393
feat: adaptive layout system — size-aware regions, crisp font ladders, image fitting#393ChuckBuilds wants to merge 10 commits into
Conversation
Add src/adaptive_layout.py — opt-in core helpers so plugins render legibly on any panel size without hand-tuned per-display layouts: - Region: integer rect algebra (bands/columns/weighted splits/centering) that partitions space so text bands can't overlap by construction - Font ladders: ordered (family, size) steps known to render crisply (LADDER_GRID: X11 BDFs at native sizes; LADDER_ARCADE: PressStart2P at 8px multiples) — fitting walks the ladder instead of scaling pixel fonts fractionally - LayoutContext: breakpoint tiers, geometry scale vs. a declared design size, and cached fit_text/fit_lines/font_for_rows queries Generalizes the three patterns proven in the field: f1-scoreboard's scale factor, masters-tournament's tiers, baseball-scoreboard's font fallback ladder. Wiring: BasePlugin gains a lazy .layout property and draw_fit(); FontManager gains get_native_bdf_size() and a cache_generation counter; manifest schema gains display.design_size and requires.display_size max_width/max_height; 96x48 joins DEFAULT_TEST_SIZES; the bounds-check harness records negative-coordinate draws; TextHelper's broken measurement helpers are fixed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add src/adaptive_images.py — the image counterpart to fit_text: - fit_image(img, box, mode=contain|cover|fill_height|stretch, crop_to_ink, anchor, resample, upscale) promoting the proven plugin patterns (football's crop-to-ink fill-height logos, masters' cover crop + NEAREST flags, static-image's letterbox). Upscales by default — thumbnail()'s downscale-only behavior is why imagery stays tiny on big panels. - draw_fitted_image() pastes aligned within a Region with alpha mask. - One central Pillow>=9.1 RESAMPLE shim replacing ~15 plugin copies. LayoutContext.fit_image() caches results per (identity, box size, options) with a 64-entry LRU; id()-keyed entries pin the source image. BasePlugin.draw_image() is the one-liner adoption path beside draw_fit. Composites in adaptive_layout.py: Region.offset() (user x/y-offset passthrough), scoreboard_regions() (the two-logos-plus-score card math duplicated across six sports plugins, logo_slot = min(H, W//2)), and media_row() (art-left/text-right). Fix LogoHelper's size-blind cache key (stale sizes on panel change); deprecation note on dead image_utils.py. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…allery Quality gates for adaptive layout: - fill_metrics()/check_scale_up() in the safety harness: overflow catches content too big for a panel, but nothing caught content that stays tiny on panels >= 2x the plugin's declared design size. The check measures lit-content extents and warns (or fails, when a plugin opts into "fill_check": "strict" in test/harness.json) below 50% coverage on the doubled axis. Warn-only by default so no existing plugin breaks. - harness.json "variants": extra runs with config overlays and their own golden dirs, so an opt-in mode (e.g. layout_mode: adaptive) is golden- tested beside the classic default. check_plugin.py loops base + variants and labels variant results mode@name. - Dev preview server: GET /api/sizes (harness size sample), POST /api/render-matrix (render at up to 12 sizes in one call), size-preset dropdown, and an "All Sizes" side-by-side gallery in the preview UI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… warning Discoverability: re-export the adaptive layout/image API from src.common (the blessed-helpers package plugin authors already know) — canonical paths stay src.adaptive_layout / src.adaptive_images so nothing breaks. Document it in src/common/README.md and cross-link ADAPTIVE_LAYOUT.md from the developer docs authors actually read (quick reference, API reference, advanced dev, font manager, dev preview, plugin dev guide); ADAPTIVE_LAYOUT.md gains adaptive-images, composite-layouts and preserving-user-customization sections. Compat: PluginLoader now logs one advisory warning (never raises) when a plugin's manifest declares a min LEDMatrix version newer than the running core, checking the min_ledmatrix_version / requires.* / versions[] spellings found in the wild. Guarded against stale core version numbers. src/__init__.py __version__ bumped 1.0.0 -> 3.1.0 to match the latest release tag (v3.1.0) — it had never been updated and the compat check needs a truthful number. NOTE: verify this matches the intended release numbering before the next tag. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… blurry PIL antialiases TTF outlines by default; a 'pixel-style' font only rasterizes without antialiasing at specific sizes (for PressStart2P: exact multiples of its 8px design grid). A ladder rung at an unverified size silently renders blurry on an LED panel — this exact bug shipped in both text-display's and football-scoreboard's custom TTF ladders (non-8-multiple PressStart2P sizes, and '5by7.regular'/'4x6-font' at sizes that were never actually crisp). measure_font_crispness(font, sample_text) renders the sample and reports the fraction of ink-bbox pixels that are neither pure black nor pure white. BDF fonts (real bitmaps) always score 0.0; TTF ladders should be verified against this before shipping — see the new TestFontFitting::test_ladder_arcade_is_crisp pattern. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ays-maximize fit_text always picks the largest ladder rung that fits its box. That's right when an element owns dedicated space, but wrong when several independently-fitted elements need to stay visually harmonious as the panel grows: a score's box might have generous room while a neighboring logo scales by a fixed geometry factor via px() — fit_text lets the score balloon out of proportion (even overlapping the logo) even though its individual pick is technically correct. fit_text_proportional(text, box, base_size_px, ladder) instead targets base_size_px * self.scale (the same scale factor px() already uses), picking the nearest ladder rung at or below that target, still capped to what fits the box, floored at the smallest rung when the target is below every rung. Refactored the shared largest-that-fits/ellipsize walk into _walk_ladder() so fit_text and fit_text_proportional don't duplicate it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ride self.scale (min(width_ratio, height_ratio)) is the right conservative default for anything whose aspect ratio matters, but a caller whose surrounding composition already scales along a single axis — e.g. football-scoreboard's logo_slot = min(height, width // 2), which tracks height alone — needs text sized the same way, or it reads as under-scaled next to logos that grew on a panel that only got taller (128x32 -> 128x64: self.scale stays 1.0 since width didn't grow, but logos still double). fit_text_proportional(..., scale=None) now accepts an explicit override; None keeps the existing self.scale default. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ect ratios logo_slot = min(height, width // 2) has a blind spot: at exactly 2:1 aspect ratio (width == 2 * height -- a very common shape: two, four, or more square modules stacked into a taller panel) width // 2 and height are equal, so the two logo slots claim the ENTIRE width and leave zero pixels for a center column, no matter how large the panel gets. Not a 'small panel' problem -- 96x48, 128x64, and 256x128 (all exactly 2:1) hit it identically, while the 128x32 design baseline and panels like 192x48 or 256x32 never do, because height is already the tighter constraint there. Two new parameters fix it in the one shared helper every scoreboard-style plugin composes through: - min_center_fraction / min_center_design_px reserve at least max(width * fraction, design_px * ctx.scale) for the center column, capping logo_slot further when needed. The scaled design-px term matters on small panels where a flat fraction alone reserves too little absolute space. - score_bleed_fraction extends the score's own fit box (not the logo slots themselves) a controlled amount into each side -- the same way real broadcast scoreboards let a big score number's edges cross into the team marks flanking it. Without this the reserve alone can still be too narrow for a short score to render without truncating. score_area is now genuinely narrower than the full card width (previously identical to status_band/detail_band, which still span the full width and overlay the logos -- short text there was never the problem). Verified against the full harness size spread: a real game score like '17-21' never needs ellipsis at any tested 2:1-or-tighter aspect ratio (test_score_never_needs_ellipsis_for_a_short_score), and wide panels (128x32/192x48/256x32-style) are provably unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 6 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds an opt-in adaptive layout and image-fitting system for plugins, exposes it through ChangesAdaptive layout system
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
| display_manager=display_manager, | ||
| cache_manager=cache_manager, | ||
| plugin_manager=plugin_manager, | ||
| install_deps=False, |
| mock_data, width, height, skip_update) | ||
| except Exception as e: | ||
| return jsonify({'error': f'Failed to load plugin: {e}'}), 500 | ||
| return jsonify(result) |
| 'render_time_ms': 0, | ||
| 'errors': [f'Failed to load plugin: {e}'], | ||
| 'warnings': []}) | ||
| return jsonify({'results': results}) |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| ErrorProne | 2 high |
🟢 Metrics 234 complexity · 2 duplication
Metric Results Complexity 234 Duplication 2
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.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/font_manager.py (1)
106-121: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
cache_generationisn't bumped by every cache-invalidating path, breaking its own contract.The docstring at Line 107-108 says the counter is "bumped whenever cached font objects are invalidated," but only
reload_configincrements it.clear_cache()(and its callersset_override,remove_override,add_font,remove_font) also clearfont_cache/metrics_cachewithout bumpingcache_generation.BasePlugin.layout(src/plugin_system/base_plugin.py) uses this exact counter to decide whether to rebuild its memoizedLayoutContext(and thus its_fit_cache), so a running plugin's adaptive-layout font-fit results will silently go stale after any font-override/add/remove operation from the web UI, only refreshing on process restart.Bumping it inside
clear_cache()itself would cover all four call sites in one place:🐛 Proposed fix (outside the selected range, in `clear_cache()`)
def clear_cache(self): """Clear font and metrics cache.""" self.font_cache.clear() self.metrics_cache.clear() self.cache_generation += 1 logger.info("Font cache cleared")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/font_manager.py` around lines 106 - 121, Update FontManager.clear_cache() to increment cache_generation whenever it clears font_cache and metrics_cache, preserving the existing cache-clearing behavior and ensuring callers such as set_override, remove_override, add_font, and remove_font invalidate dependent caches consistently; avoid adding separate increments in those callers or reload_config.
🧹 Nitpick comments (5)
src/common/logo_helper.py (1)
75-88: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winGood fix — extract the shared default-size computation.
Computing
max_width/max_heightbefore the cache lookup correctly makes the cache size-qualified. However this same "default todisplay_* * 1.5" logic now exists in three places: here,_resize_logo(Line 228-231), and_create_placeholder_logo(Line 286-289). Consider extracting a small private helper (e.g._effective_max_size(max_width, max_height)) to avoid future divergence.♻️ Proposed refactor
def _effective_max_size(self, max_width: Optional[int], max_height: Optional[int]) -> Tuple[int, int]: """Resolve default max dimensions (display size * 1.5) when unset.""" if max_width is None: max_width = int(self.display_width * 1.5) if max_height is None: max_height = int(self.display_height * 1.5) return max_width, max_heightThen call
max_width, max_height = self._effective_max_size(max_width, max_height)inload_logo,_resize_logo, and_create_placeholder_logo.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/common/logo_helper.py` around lines 75 - 88, Extract the repeated default-dimension logic into a private helper such as _effective_max_size, preserving the display_width/display_height multiplied by 1.5 behavior for unset values. Update load_logo, _resize_logo, and _create_placeholder_logo to resolve max_width and max_height through this helper before using them.src/plugin_system/base_plugin.py (1)
216-251: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUpdate the plugin development guide for
draw_image()docs/PLUGIN_DEVELOPMENT_GUIDE.mdstill says there is nodraw_image()helper, butBasePlugin.draw_image()is now the supported adaptive-layout API, so the guidance should be aligned.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/plugin_system/base_plugin.py` around lines 216 - 251, Update docs/PLUGIN_DEVELOPMENT_GUIDE.md to document BasePlugin.draw_image() as the supported adaptive-layout image helper. Remove or revise guidance claiming no draw_image() helper exists, and describe its fit/alignment behavior and relevant options consistently with the method’s signature.Source: Coding guidelines
src/adaptive_layout.py (1)
32-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnused imports flagged by static analysis in two new files. Both
field(fromdataclasses) inadaptive_layout.pyandUnion(fromtyping) inadaptive_images.pyare imported but never referenced — all type annotations useAny,Tuple, orOptionalinstead.
src/adaptive_layout.py#L32: removefieldfrom thedataclassesimport.src/adaptive_images.py#L28: removeUnionfrom thetypingimport.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/adaptive_layout.py` at line 32, Remove the unused field import from the dataclasses import in src/adaptive_layout.py (line 32), and remove the unused Union import from the typing import in src/adaptive_images.py (line 28); leave all referenced imports unchanged.Source: Linters/SAST tools
src/plugin_system/testing/harness.py (1)
328-341: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider
histogram()for the ink-ratio calculation.
sum(1 for p in lit.getdata() if p)iterates every pixel in Python. After thresholding,litis binary (0 or 255), solit.histogram()[255]gives the lit count in a single C-level call — faster and more idiomatic, with no generator overhead. The difference is negligible for small LED matrix images but aligns with resource-conscious practices.♻️ Optional refactor
- ink = sum(1 for p in lit.getdata() if p) / (image.width * image.height) + ink = lit.histogram()[255] / (image.width * image.height)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/plugin_system/testing/harness.py` around lines 328 - 341, Update the ink calculation in fill_metrics to use lit.histogram()[255] instead of iterating over lit.getdata(), preserving the existing normalization by image.width * image.height and the returned metrics.src/plugin_system/testing/mocks.py (1)
164-170: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winReuse the shared fallback
FontManagerhere.FontManager({})eagerly scansassets/fontsand reloads overrides in__init__, so eachMockPluginManager()repeats that work;BasePlugin._get_font_manager()already falls back to the module-level singleton.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/plugin_system/testing/mocks.py` around lines 164 - 170, Update MockPluginManager’s font_manager initialization to reuse the shared module-level fallback FontManager instead of constructing FontManager({}) for every instance. Preserve the existing exception fallback behavior and ensure BasePlugin._get_font_manager() continues using the singleton when no manager is available.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/FONT_MANAGER.md`:
- Around line 3-6: Update the “Picking a size automatically” documentation to
scope self.layout.fit_text(...) explicitly to BasePlugin authors, or explain how
FontManager users obtain the required layout context; avoid directing generic
manager readers to an unavailable self.layout attribute.
In `@docs/PLUGIN_DEVELOPMENT_GUIDE.md`:
- Around line 5-8: Revise the “Rendering guidance” text in
PLUGIN_DEVELOPMENT_GUIDE.md to clarify that dynamic display sizing and adaptive
layouts apply only to plugins adopting the adaptive APIs. Explicitly preserve
classic rendering behavior as the default rather than implying all plugins must
migrate.
In `@scripts/dev_server.py`:
- Around line 277-287: Update the request handlers around _parse_render_request,
_render_once, and api_render_matrix to catch all expected parsing/rendering
exceptions, log the full exceptions server-side, and return only generic error
messages to API clients. Remove direct str(e), f-string exception interpolation,
and any raw exception text carried in result responses, while preserving the
existing HTTP status codes and successful response behavior.
---
Outside diff comments:
In `@src/font_manager.py`:
- Around line 106-121: Update FontManager.clear_cache() to increment
cache_generation whenever it clears font_cache and metrics_cache, preserving the
existing cache-clearing behavior and ensuring callers such as set_override,
remove_override, add_font, and remove_font invalidate dependent caches
consistently; avoid adding separate increments in those callers or
reload_config.
---
Nitpick comments:
In `@src/adaptive_layout.py`:
- Line 32: Remove the unused field import from the dataclasses import in
src/adaptive_layout.py (line 32), and remove the unused Union import from the
typing import in src/adaptive_images.py (line 28); leave all referenced imports
unchanged.
In `@src/common/logo_helper.py`:
- Around line 75-88: Extract the repeated default-dimension logic into a private
helper such as _effective_max_size, preserving the display_width/display_height
multiplied by 1.5 behavior for unset values. Update load_logo, _resize_logo, and
_create_placeholder_logo to resolve max_width and max_height through this helper
before using them.
In `@src/plugin_system/base_plugin.py`:
- Around line 216-251: Update docs/PLUGIN_DEVELOPMENT_GUIDE.md to document
BasePlugin.draw_image() as the supported adaptive-layout image helper. Remove or
revise guidance claiming no draw_image() helper exists, and describe its
fit/alignment behavior and relevant options consistently with the method’s
signature.
In `@src/plugin_system/testing/harness.py`:
- Around line 328-341: Update the ink calculation in fill_metrics to use
lit.histogram()[255] instead of iterating over lit.getdata(), preserving the
existing normalization by image.width * image.height and the returned metrics.
In `@src/plugin_system/testing/mocks.py`:
- Around line 164-170: Update MockPluginManager’s font_manager initialization to
reuse the shared module-level fallback FontManager instead of constructing
FontManager({}) for every instance. Preserve the existing exception fallback
behavior and ensure BasePlugin._get_font_manager() continues using the singleton
when no manager is available.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9bb2abf3-8b18-4887-8367-771f799d6c38
📒 Files selected for processing (31)
docs/ADAPTIVE_LAYOUT.mddocs/ADVANCED_PLUGIN_DEVELOPMENT.mddocs/DEVELOPER_QUICK_REFERENCE.mddocs/DEV_PREVIEW.mddocs/FONT_MANAGER.mddocs/PLUGIN_API_REFERENCE.mddocs/PLUGIN_DEVELOPMENT_GUIDE.mdschema/manifest_schema.jsonscripts/check_plugin.pyscripts/dev_server.pyscripts/templates/dev_preview.htmlsrc/__init__.pysrc/adaptive_images.pysrc/adaptive_layout.pysrc/common/README.mdsrc/common/__init__.pysrc/common/logo_helper.pysrc/common/text_helper.pysrc/font_manager.pysrc/image_utils.pysrc/plugin_system/base_plugin.pysrc/plugin_system/plugin_loader.pysrc/plugin_system/testing/bounds_display_manager.pysrc/plugin_system/testing/harness.pysrc/plugin_system/testing/loading.pysrc/plugin_system/testing/mocks.pysrc/plugin_system/testing/sizes.pytest/test_adaptive_images.pytest/test_adaptive_layout.pytest/test_harness_fill.pytest/test_loader_compat_warning.py
- docs: scope the self.layout note to BasePlugin subclasses (others build a LayoutContext directly) and make explicit that adaptive layout is opt-in — classic rendering stays unless a plugin adopts the APIs. - dev_server: broaden the render-request catch (a bad manifest.json now returns a clean 400 instead of an unhandled 500) and stop echoing raw exception text in the loader-failure responses — full tracebacks go to the dev server's console log instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam
Summary
Core infrastructure for plugins that scale to any panel size instead of being authored for one:
src/adaptive_layout.py—Regionrect algebra (bands/columns/splits/offset),LayoutContextwith breakpoint tiers and a geometry scale factor vs. the manifest'sdisplay.design_size, and font fitting against verified-crisp font ladders (measure_font_crispnessproves each rung renders with zero antialiasing; PressStart2P is only crisp at exact multiples of 8px — classic football's fixed 10px score has always been 74.6% antialiased, a pre-existing surprise this tooling caught).fit_text_proportional— sizes text relative to its classic design size (nearest crisp rung), not "largest that fits its box," so elements can't balloon out of proportion to their neighbors; optional axis-specificscaleoverride for compositions that scale by height alone.scoreboard_regions()/media_row()— shared composition helpers.scoreboard_regionsfixes thelogo_slot = min(h, w//2)blind spot at exactly 2:1 aspect ratios (96x48, 128x64, 256x128 — where the logos mathematically claimed the whole card, leaving zero pixels for the score) with a guaranteed center reserve + broadcast-style score bleed. Fixed once here; adopting plugins need zero code changes.src/adaptive_images.py—fit_image(contain/cover/fill_height/stretch, crop-to-ink, NEAREST/LANCZOS) + size-keyed LRU cache onLayoutContext; retires ~15 per-plugin copies of the thumbnail/resample idiom and the size-blind logo caches.BasePlugin.layout/draw_fit/draw_image— one-liner adoption path.harness.json), configvariantsfor dual classic/adaptive goldens, dev-server multi-size gallery.Verification
test/test_adaptive_layout.py+test_adaptive_images.pysuites; all existing suites pass.🤖 Generated with Claude Code
https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam
Summary by CodeRabbit
New Features
Documentation
Bug Fixes