fix(composer): close binding_source code injection, line-align, and project_root UnboundLocalError

Three findings from CodeRabbit's review of 6c23994b, all verified against
current code before fixing:

- manager.py.j2 interpolated binding.source unescaped into a Python comment
  (`pass  # dynamic_text binding_source "{{ el.binding_source }}" draws
  nothing`). A source string with a newline broke out of the comment; a
  crafted payload produces a clean, ast.parse-valid `import os` in the
  generated plugin (confirmed against the pre-fix template). This is now a
  fixed literal comment that never interpolates the value. Live now that
  composer_bp is registered. CWE-94.
- _alignElement moved a line's x0 (or y0) to the new position but left x1
  (or y1) behind, so aligning a line changed its shape instead of moving
  it. Both endpoints now translate by the same delta.
- web_interface/app.py only assigned project_root inside the relative-path
  branch of the plugins_dir resolution. An absolute plugin_system.plugins_
  directory (a supported config value) hit UnboundLocalError importing the
  module at all, since SchemaManager/composer_bp use project_root further
  down. Now assigned unconditionally before the branch.

Also extends BOUND_TYPES coverage in composer-app.js (_isBound,
removeConfigVar, _validateBeforeExport) from dynamic_text/progress_bar to
all six element types that carry a binding object (countdown, pips,
sparkline, gauge too) -- found by direct code reading against
ELEMENT_DEFAULTS in composer-canvas.js, not from a review comment. Without
it, those four types could export with an unbound config key with no
validation error, and deleting a config var they used gave no warning.

All four fixes have mutation-checked regression tests (fail against the
reverted code, pass with the fix): test_binding_source_cannot_break_out_of_the_comment_it_lands_in,
test_align_translates_both_line_endpoints_not_just_the_start,
test_app_plugins_dir_resolution.py, test_binding_checks_cover_every_bound_element_type.

Full suite: 4365 passed, 58 skipped, 2 failed -- both the pre-existing
Europe/Kiev/Asia/Calcutta tzdata-alias gap on this sandbox, identical on
origin/main, unrelated to this change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

docs/SCROLL_PERFORMANCE.md

@@ -1,266 +0,0 @@
-# Scroll Performance
-
-How scrolling is paced on this hardware, what was wrong with it, and how to
-configure a plugin so its marquee is smooth.
-
-Measured on a Raspberry Pi 4 driving a 2×128×64 chain (256×64 logical) at
-`limit_refresh_rate_hz: 100`. Numbers below come from that panel.
-
-| | before | after |
-|---|---|---|
-| scroll frame rate | 44–46 fps | **100 fps, locked** |
-| frames ≥ 45 ms | 14–17% | none observed |
-| dominant frame time | 20 ms | **10 ms** |
-| disk cache write (~1 MB) | 14.8 ms | **5.4 ms** |
-
----
-
-## The one rule that matters
-
-**Motion is smooth when the strip advances a whole number of pixels per panel
-refresh.**
-
-Advancing one pixel per refresh on a 100 Hz panel gives 100 px/s. Slower crisp
-speeds come from holding each frame for several refreshes -- 50 px/s is one
-pixel every second refresh -- which is covered under *Choosing a speed* below.
-A speed that lands on no such combination has to do one of two bad things:
-
-- **blend** two adjacent columns to render a half-step — on pixel-font text
-  this alternates crisp and smeared frames and reads as shimmer, or as the
-  text jumping a pixel ahead of itself;
-- **repeat** a frame — the strip stands still, then jumps, which reads as
-  judder.
-
-Neither is tunable away. Pick a speed that divides evenly.
-
-`src.common.scroll_config` solves this for you: `configure()` snaps a requested
-speed to the nearest one the panel can actually show in whole pixels, and
-`scripts/scroll_speeds.py` prints the full ladder for your hardware.
-
-## Choosing a speed
-
-The crisp speeds are not a fixed list -- they depend on how fast *your* panel
-refreshes, which depends on its size, `pwm_bits`, `gpio_slowdown` and the Pi
-model. A Pi Zero driving a long chain has a completely different set of good
-speeds from a Pi 4 driving a short one.
-
-```bash
-# what can this panel do? (reads your configured refresh rate)
-python3 scripts/scroll_speeds.py
-
-# what does it ACTUALLY manage, rather than what is configured?
-sudo systemctl stop ledmatrix
-sudo python3 scripts/scroll_speeds.py --measure
-sudo systemctl start ledmatrix
-
-# highlight the closest option to the speed you want
-python3 scripts/scroll_speeds.py --want 45
-
-# try one on the panel
-sudo systemctl stop ledmatrix
-sudo python3 scripts/scroll_speeds.py --demo 50
-sudo systemctl start ledmatrix
-```
-
-Sample ladder for a 100 Hz panel:
-
-```
-  20.0 px/s  (1px every 5 refreshes =  20.0 fps, slightly stepped)
-  25.0 px/s  (1px every 4 refreshes =  25.0 fps, slightly stepped)
-  33.3 px/s  (1px every 3 refreshes =  33.3 fps, smooth)
-  50.0 px/s  (1px every 2 refreshes =  50.0 fps, smooth)
-  66.7 px/s  (2px every 3 refreshes =  33.3 fps, smooth)
- 100.0 px/s  (1px every 1 refresh  = 100.0 fps, smooth)
-```
-
-### How a slow speed stays crisp
-
-`SwapOnVSync(canvas, framerate_fraction)` holds each frame for N panel
-refreshes. **The panel keeps refreshing at its full rate either way**, so
-holding a frame costs nothing in flicker -- it only changes how often a *new*
-image is presented. That is what allows 50 px/s to be one whole pixel every
-second refresh, instead of half a pixel every refresh (which has no good
-rendering, only a choice between blur and judder).
-
-`scroll_config.configure()` snaps the requested speed to the nearest entry on
-the ladder and reports the hold that speed needs. It does **not** apply the
-hold: the hold belongs to a scroll, not to a plugin's lifetime, and plugins
-share one display manager -- one set at construction is reset the moment any
-other plugin finishes scrolling. Apply it yourself when the scroll starts:
-
-```python
-settings = scroll_config.configure(
-    self.scroll_helper,
-    plugin_config=self.config,
-    global_config=self.global_config,
-    display_manager=self.display_manager,   # supplies the panel refresh rate
-)
-
-# ...then, each time this plugin begins scrolling:
-self.display_manager.set_scrolling_state(True, frame_hold=settings.frame_hold)
-```
-
-Passing `display_manager` only lets `configure` read the true refresh rate from
-`display.hardware`, which a plugin config cannot see. Skipping the
-`set_scrolling_state` call is the mistake that matters: the speed still
-resolves, but the panel keeps presenting a new frame every refresh, so a slow
-snapped speed falls back to fractional pixels. Pass `snap_to_crisp=False` to
-keep an exact requested speed and accept the artefacts.
-
-Speeds slower than about 20 px/s are stepped no matter what, because a 1-pixel
-advance at 20 fps is simply a coarse increment. That is the pixel pitch, not a
-software limit; the only way to move in smaller increments is sub-pixel
-blending, which this display does not tolerate (see above).
-
-## Configuring a plugin
-
-Use the shared resolver rather than reading config keys yourself:
-
-```python
-from src.common import scroll_config
-
-settings = scroll_config.configure(
-    self.scroll_helper,
-    plugin_config=self.config,
-    global_config=self.global_config,
-    refresh_hz=scroll_config.refresh_hz_from_config(self.global_config),
-    plugin_logger=self.logger,
-)
-```
-
-It resolves every config shape in one place, applies the speed, and returns
-what it did. Precedence, highest first:
-
-1. `display_options.scroll_speed` + `scroll_delay` — **the recommended form**
-2. `display.scroll_speed` + `scroll_delay` — deprecated shape
-3. `scroll_speed` + `scroll_delay` at the root — legacy flat
-4. `scroll_pixels_per_second` — deprecated
-5. the global `display` block
-6. the built-in default (100 px/s)
-
-`scroll_speed` is pixels per frame and `scroll_delay` is the frame period in
-seconds, so the pair means `scroll_speed / scroll_delay` px/s. The recommended
-config for a 100 Hz panel:
-
-```json
-"display_options": { "scroll_speed": 1.0, "scroll_delay": 0.01 }
-```
-
-### Why the deprecated key ranks below the explicit pair
-
-Because some plugins give `scroll_pixels_per_second` a **schema default**, and
-schema defaults are merged into plugin config. Ranking it above the pair means
-it is always present and always wins, so the documented settings become
-unreachable. That is a real, shipped bug — see
-[ledmatrix-plugins#408](https://github.com/ChuckBuilds/ledmatrix-plugins/issues/408).
-
-If you are writing a plugin: do not give a deprecated key a schema default.
-
-## What was actually wrong
-
-Four independent faults, each found by measurement.
-
-### 1. The frame loop slept on top of a wait it had already done
-
-`display_controller.py` ran the high-FPS loop as `render → SwapOnVSync (blocks
-to the panel's refresh) → time.sleep(0.008) → plugin ticks`. The sleep was
-unconditional and added to a wait that had already happened. Render work
-measured ~4 ms, so each iteration cost ~12 ms against a 10 ms refresh grid —
-every swap missed a refresh and landed on the next one. The loop settled at
-exactly 50 fps while asking for 125, with no headroom, so ~14% of frames
-slipped a further refresh.
-
-Now the loop sleeps only the remainder of the frame budget, with a 1 ms floor
-so plugin threads still get the GIL.
-
-### 2. `SwapOnVSync` held the GIL while blocking
-
-The rgbmatrix binding declares it without `nogil` (unlike `SetPixel`, `Clear`
-and `Fill` immediately above it in `cppinc.pxd`), so the render thread held the
-GIL for the entire vsync wait — most of every frame. Background threads were
-starved into long uninterruptible bursts; a 1.5 MB API response costs ~17 ms to
-parse and ~18 ms to re-encode for the cache, and `json.raw_decode` cannot be
-preempted mid-document. Those bursts are what the render loop then waited on.
-
-Fixed by rebuilding the binding: `scripts/build_rgbmatrix_nogil.sh`.
-
-### 3. Sub-pixel blending was wrong for this display
-
-Enabling it made things worse, not better — see the rule at the top. It is off
-by default and only Vegas mode opts in via `set_sub_pixel_scrolling(True)`.
-
-### 4. Frame-based stepping raced the vsync clock
-
-Frame-based mode gated motion on a wall clock at `1/scroll_delay` steps per
-second. Plugins set `scroll_delay` to the frame period, which puts that
-comparison exactly on its own threshold: a frame arriving a hair early moved
-zero pixels and rendered an identical frame, which dirty-tracking skipped, so
-it returned in ~2 ms and the beat repeated. No `scroll_delay` value tunes this
-out — a shorter delay just trades stalled frames for periodic double-steps.
-
-`ScrollHelper` now accumulates elapsed time in both modes at the same
-configured speed, so position stays proportional to real time.
-
-## Diagnosing a juddery scroller
-
-**`Avg FPS` will lie to you.** It is a 100-frame moving average, and a 2 ms
-duplicate frame plus a 21 ms double-wait average to exactly 10 ms. A ticker
-that is stalling on half its frames still reports a healthy `100.0`.
-
-Look at the **distribution** instead:
-
-```bash
-journalctl -u ledmatrix --since "-10min" --no-pager \
-  | grep -oE "Frame time: [0-9.]+ms" | awk '{print $3}' | sed 's/ms//' \
-  | awk '{printf "%.0f\n", $1}' | sort -n | uniq -c
-```
-
-Reading it, on a 100 Hz panel:
-
-| you see | it means |
-|---|---|
-| everything at 10 ms | healthy |
-| a mode at ~2 ms | **duplicate frames** — the swap was skipped because the image did not change. The scroller is advancing less than one pixel per frame. |
-| a mode at 20/30/50 ms | frames missing refreshes — per-frame work is overrunning, or a background thread is holding the GIL |
-| `Avg FPS` above 100 | duplicates present, unless the scroll cycle has completed and is idling |
-
-Then confirm what the plugin actually loaded — config edits do not always reach
-the running code:
-
-```bash
-journalctl -u ledmatrix --since "-5min" --no-pager | grep -iE "px/s|px/frame"
-```
-
-If a plugin logs its scroll config **twice** with different modes, the second
-line is what is running.
-
-## Rebuilding the binding
-
-```bash
-bash scripts/build_rgbmatrix_nogil.sh              # build into a scratch dir
-sudo bash scripts/build_rgbmatrix_nogil.sh --install
-sudo bash scripts/build_rgbmatrix_nogil.sh --rollback
-```
-
-The build never touches the installed module. `--install` backs up the original
-to `~/rgbmatrix-core.so.ORIGINAL` first, and rolls back automatically if the
-service does not come back healthy. Requires `build-essential`; Cython is
-installed into a cached venv under `~/.cache/ledmatrix-cython`.
-
-Re-run it after upgrading `rpi-rgb-led-matrix`, since a library upgrade
-replaces the patched binding.
-
-## Faster JSON
-
-`src/cache/disk_cache.py` uses `orjson` when it is importable and falls back to
-the stdlib otherwise, so it is optional:
-
-```bash
-sudo pip3 install --break-system-packages orjson
-```
-
-Encoding is where it pays — about 7× on this hardware. Decoding gains far less
-(~1.3× on large payloads) because the cost there is building Python objects,
-not scanning text. That is also why moving parsing to a subprocess does not
-help: `pickle.loads` of the same payload costs 8.1 ms against `json.loads` at
-10.9 ms, so the work just moves rather than disappearing.

plugin-repos/starlark-apps/manager.py

@@ -681,18 +681,12 @@ class StarlarkAppsPlugin(BasePlugin):
                 if app.is_enabled() and app.should_render(current_time):
                     self._render_app(app, force=False)
 
-    def display(self, force_clear: bool = False) -> bool:
+    def display(self, force_clear: bool = False) -> None:
         """
         Display current Starlark app.
 
         This method is called during the display rotation.
         Displays frames from the currently active app.
-
-        Returns False when there is no app to show -- which is the state of
-        every install without Pixlet, and of a fresh one before any app is
-        added. The display controller only skips a mode on a boolean False
-        (it checks isinstance(result, bool)), so returning None held a black
-        panel for the full display_duration instead of rotating on.
         """
         try:
             if force_clear:
@@ -705,24 +699,20 @@ class StarlarkAppsPlugin(BasePlugin):
             if not self.current_app:
                 # No apps available
                 self.logger.debug("No Starlark apps to display")
-                return False
+                return
 
             # Render app if needed
             if not self.current_app.frames:
                 success = self._render_app(self.current_app, force=True)
                 if not success:
                     self.logger.error(f"Failed to render app: {self.current_app.app_id}")
-                    return False
+                    return
 
-            # Display current frame. The result is propagated: a failed frame
-            # update is not a displayed frame, and returning True regardless
-            # told the controller the mode had rendered, so it held the dead
-            # frame for the whole display_duration instead of rotating on.
-            return self._display_frame()
+            # Display current frame
+            self._display_frame()
 
         except Exception as e:
             self.logger.error(f"Error displaying Starlark app: {e}")
-            return False
 
     def _select_next_app(self) -> None:
         """Select the next enabled app for display."""
@@ -845,13 +835,10 @@ class StarlarkAppsPlugin(BasePlugin):
             self.logger.error(f"Error loading frames for {app.app_id}: {e}")
             return False
 
-    def _display_frame(self) -> bool:
-        """Display the current frame of the current app.
-
-        :returns: whether a frame actually reached the display manager.
-        """
+    def _display_frame(self) -> None:
+        """Display the current frame of the current app."""
         if not self.current_app or not self.current_app.frames:
-            return False
+            return
 
         try:
             current_time = time.time()
@@ -869,11 +856,8 @@ class StarlarkAppsPlugin(BasePlugin):
                 )
                 self.current_app.last_frame_time = current_time
 
-            return True
-
         except Exception as e:
             self.logger.error(f"Error displaying frame: {e}")
-            return False
 
     def install_app(self, app_id: str, star_file_path: str, metadata: Optional[Dict[str, Any]] = None, assets_dir: Optional[str] = None) -> bool:
         """

requirements.txt

@@ -55,18 +55,6 @@ packaging>=23.0,<27.0
 #                     range as a hard dependency — keep the two in sync.
 #   pip install 'psutil>=6.0.0,<7.0.0'
 #
-# orjson            — faster JSON for the disk cache
-#                     (src/cache/disk_cache.py). Encoding a ~1MB cache
-#                     record drops from ~12ms to ~1.6ms on a Pi 4, which
-#                     matters because that work holds the GIL and stalls
-#                     the render thread mid-scroll. Falls back to the
-#                     stdlib json when missing — see docs/SCROLL_PERFORMANCE.md.
-#                     The 3.11.6 floor is CVE-2025-67221: orjson.dumps did not
-#                     limit recursion on deeply nested documents, and the disk
-#                     cache encodes payloads parsed straight from third-party
-#                     APIs. 3.11.6 covers the Python range above.
-#   pip install 'orjson>=3.11.6,<4.0'
-#
 # Flask-Limiter     — request rate limiting in web_interface/app.py
 #                     (accidental-abuse protection, not security). The
 #                     web interface starts without rate limiting when

scripts/audit_render_path.py

@@ -1,159 +0,0 @@
-#!/usr/bin/env python3
-"""Find blocking work reachable from a plugin's render path.
-
-`display()` runs on the render thread. Anything slow reached from it stalls the
-panel for its whole duration, and on a vsync-paced loop that is immediately
-visible: a single 15ms call on a 100Hz panel drops a frame, and a network round
-trip freezes the marquee outright.
-
-This has bitten twice already. odds-ticker called `_has_live_games()` every
-frame, whose slow path read the scoreboard cache from disk and parsed JSON per
-enabled league -- one stalled frame every few minutes. soccer-scoreboard timed
-out inside `update()` during a cache refresh. Both were found by staring at
-frame-time histograms, which is a slow way to find a bug that is visible in the
-source.
-
-The audit walks the call graph from `display()` through same-class `self.*`
-methods and reports anything that reaches a known-blocking API. It is a
-heuristic, not a proof: it cannot see through indirection, and a hit is not
-automatically a bug -- a call guarded by an interval check may be fine. It is a
-list of places worth a human look.
-
-    python3 scripts/audit_render_path.py                    # all plugins
-    python3 scripts/audit_render_path.py --dir plugin-repos # a specific tree
-    python3 scripts/audit_render_path.py --plugin odds-ticker
-"""
-from __future__ import annotations
-
-import argparse
-import ast
-import sys
-from pathlib import Path
-
-#: Calls that can block for longer than a frame. Matched on the attribute or
-#: function name, so `requests.get`, `self.session.get` and a bare `get` on a
-#: requests-ish object all register.
-BLOCKING = {
-    "get": "network or cache read",
-    "post": "network",
-    "put": "network",
-    "request": "network",
-    "urlopen": "network",
-    "read": "I/O",
-    "open": "file I/O",
-    "load": "JSON/file parse",
-    "loads": "JSON parse",
-    "dump": "file write",
-    "dumps": "serialise",
-    "sleep": "sleep on the render thread",
-    "run": "subprocess",
-    "check_output": "subprocess",
-    "connect": "network",
-    "download_logo": "network",
-    "_fetch": "fetch",
-}
-
-#: Names that make a hit far more likely to matter.
-HIGH_SIGNAL = ("requests", "urllib", "session", "cache_manager", "subprocess",
-               "socket", "http")
-
-RENDER_ENTRY = "display"
-
-
-class Analyzer:
-    def __init__(self, tree: ast.AST):
-        self.methods: dict[str, ast.FunctionDef] = {}
-        for node in ast.walk(tree):
-            if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)):
-                self.methods.setdefault(node.name, node)
-
-    def calls_in(self, fn: ast.AST):
-        """(self-method names called, blocking hits) inside one function."""
-        self_calls, hits = set(), []
-        for node in ast.walk(fn):
-            if not isinstance(node, ast.Call):
-                continue
-            func = node.func
-            if isinstance(func, ast.Attribute):
-                name = func.attr
-                base = ast.unparse(func.value) if hasattr(ast, "unparse") else ""
-                if base == "self" and name in self.methods:
-                    self_calls.add(name)
-                    continue
-                if name in BLOCKING:
-                    hits.append((name, base, BLOCKING[name], node.lineno))
-            elif isinstance(func, ast.Name) and func.id in BLOCKING:
-                hits.append((func.id, "", BLOCKING[func.id], node.lineno))
-        return self_calls, hits
-
-    def reachable_from(self, entry: str, max_depth: int = 3):
-        """Blocking hits reachable from `entry`, with the path that reaches them."""
-        if entry not in self.methods:
-            return []
-        found, seen = [], set()
-        stack = [(entry, [entry], 0)]
-        while stack:
-            name, path, depth = stack.pop()
-            if name in seen or depth > max_depth:
-                continue
-            seen.add(name)
-            self_calls, hits = self.calls_in(self.methods[name])
-            for hit in hits:
-                found.append((path, hit))
-            for callee in sorted(self_calls):
-                stack.append((callee, path + [callee], depth + 1))
-        return found
-
-
-def audit_file(path: Path):
-    try:
-        tree = ast.parse(path.read_text(encoding="utf-8"))
-    except (OSError, SyntaxError):
-        return []
-    analyzer = Analyzer(tree)
-    return analyzer.reachable_from(RENDER_ENTRY)
-
-
-def main():
-    ap = argparse.ArgumentParser(description=__doc__,
-                                 formatter_class=argparse.RawDescriptionHelpFormatter)
-    root = Path(__file__).resolve().parent.parent
-    ap.add_argument("--dir", default=str(root / "plugin-repos"))
-    ap.add_argument("--plugin", help="only this plugin directory")
-    ap.add_argument("--all-hits", action="store_true",
-                    help="include low-signal hits (open/read/load on locals)")
-    args = ap.parse_args()
-
-    base = Path(args.dir)
-    if not base.is_dir():
-        sys.exit("not a directory: %s" % base)
-
-    plugins = [base / args.plugin] if args.plugin else sorted(
-        d for d in base.iterdir() if d.is_dir())
-
-    total = 0
-    for plugin in plugins:
-        rows = []
-        for src in sorted(plugin.glob("*.py")):
-            if src.name.startswith("test_"):
-                continue
-            for path, (name, s_base, why, lineno) in audit_file(src):
-                signal = any(h in s_base.lower() for h in HIGH_SIGNAL)
-                if not signal and not args.all_hits:
-                    continue
-                rows.append((src.name, lineno, "->".join(path),
-                             ("%s.%s" % (s_base, name)) if s_base else name, why))
-        if rows:
-            total += len(rows)
-            print("\n%s" % plugin.name)
-            for fname, lineno, chain, call, why in sorted(rows):
-                print("  %s:%-5d %-34s via %s" % (fname, lineno, call + "  (" + why + ")", chain))
-
-    print("\n%d blocking call(s) reachable from display() across %d plugin(s)"
-          % (total, len(plugins)))
-    print("Heuristic: a hit guarded by an interval check may be fine. Look, do "
-          "not assume.")
-
-
-if __name__ == "__main__":
-    main()

scripts/build_rgbmatrix_nogil.sh

@@ -1,330 +0,0 @@
-#!/usr/bin/env bash
-#
-# Rebuild the rgbmatrix Python binding so it releases the GIL.
-#
-# WHY THIS EXISTS
-# ---------------
-# The upstream binding declares FrameCanvas::SwapOnVSync WITHOUT `nogil`
-# (cppinc.pxd), unlike SetPixel/Clear/Fill on the lines just above it.
-# SwapOnVSync blocks until the panel's next vertical sync -- up to a full
-# refresh period on every frame -- so the render thread was holding the GIL
-# for most of every frame. Background threads (API fetches, JSON parsing,
-# image decode) were starved into long uninterruptible bursts, which in turn
-# made the render loop miss refreshes.
-#
-# Measured on a Pi 4 driving a 2x128x64 chain at limit_refresh_rate_hz=100:
-#
-#     before   ~44 fps average, 14-17% of frames 41-53ms
-#     after    100 fps, median 10.00ms, p95 10.05ms, 0% stalls
-#
-# The per-pixel blit (SetPixelsPillow) can also release the GIL and walk the
-# Pillow buffer row-major, but that is OFF by default and you almost certainly
-# want to leave it that way. Row-major changes what a partially-written frame
-# looks like: column-major tearing shows as a vertical seam, row-major tearing
-# shows as a horizontal split between the panel's upper and lower halves. On a
-# 1/32 scan panel that reads as a one-pixel "fold" across the middle of every
-# panel -- reported on hardware, and it went away when the blit was reverted.
-# Enable with RGB_PATCH_BLIT=1 only if you have measured that you need it;
-# essentially all of the gain above comes from the SwapOnVSync change alone.
-#
-# SAFETY
-# ------
-# Builds into a scratch directory; touches the installed module only in the
-# --install step, and backs up the original first. Roll back at any time with:
-#
-#     sudo bash scripts/build_rgbmatrix_nogil.sh --rollback
-#
-# USAGE
-#     bash scripts/build_rgbmatrix_nogil.sh              # build only
-#     sudo bash scripts/build_rgbmatrix_nogil.sh --install
-#     sudo bash scripts/build_rgbmatrix_nogil.sh --rollback
-#
-set -uo pipefail
-
-# Resolve the invoking user's home, not root's. --install runs under sudo,
-# where $HOME is /root, so every default path below pointed somewhere the
-# build had never written and the install died with "no built module found".
-if [ -n "${SUDO_USER:-}" ]; then
-    OWNER_HOME="$(getent passwd "$SUDO_USER" | cut -d: -f6)"
-fi
-OWNER_HOME="${OWNER_HOME:-$HOME}"
-
-SRC_TREE="${RGB_SRC_TREE:-$OWNER_HOME/LEDMatrix/rpi-rgb-led-matrix-master}"
-BUILD_DIR="${RGB_BUILD_DIR:-$OWNER_HOME/rgbmatrix-nogil-build}"
-VENV="${RGB_CYTHON_VENV:-$OWNER_HOME/.cache/ledmatrix-cython}"
-BACKUP="${RGB_BACKUP:-$OWNER_HOME/rgbmatrix-core.so.ORIGINAL}"
-PATCH_BLIT="${RGB_PATCH_BLIT:-0}"
-
-die() { echo "FATAL: $*" >&2; exit 1; }
-
-# This script runs under `set -uo pipefail` -- no -e -- so an unchecked
-# systemctl failure is silently ignored. That matters most for `stop`: leaving
-# the old service running means cp overwrites a module the running process has
-# mapped, the following `start` succeeds as a no-op, and the health check sees
-# an active unit and reports SUCCESS for a binding that was never loaded.
-# A machine with no ledmatrix.service at all is a normal build host, so that
-# case is skipped rather than treated as a failure.
-service_present() { systemctl cat ledmatrix.service >/dev/null 2>&1; }
-
-service_do() {
-    local verb="$1"
-    if ! service_present; then
-        echo "   (no ledmatrix.service installed - skipping $verb)"
-        return 0
-    fi
-    systemctl "$verb" ledmatrix || die "systemctl $verb ledmatrix failed"
-}
-
-py_site() {
-    python3 -c 'import rgbmatrix, os; print(os.path.dirname(rgbmatrix.__file__))' 2>/dev/null
-}
-
-# The extension filename the interpreter that builds -- and then loads -- this
-# module actually uses, e.g. core.cpython-313-aarch64-linux-gnu.so. The build
-# venv is made with --system-site-packages from python3, so the two agree;
-# falling back keeps --install working when the venv has been cleaned up.
-abi_name() {
-    local py="$VENV/bin/python"
-    [ -x "$py" ] || py=python3
-    "$py" -c \
-        'import sysconfig; print("core" + sysconfig.get_config_var("EXT_SUFFIX"))' \
-        2>/dev/null
-}
-
-# Exactly the current interpreter's artifact, never merely the first one that
-# sorts. Staging copies $SRC_TREE wholesale, so a core.cpython-*.so left in the
-# source tree by an earlier build comes along for the ride; build_ext --inplace
-# only ever overwrites the current ABI's name, and a glob piped to `head -1`
-# sorts cpython-311 ahead of cpython-313. That installed a stale, unpatched
-# module as core.so while the GIL check below -- which reads the freshly
-# generated core.cpp, not the .so -- still reported success.
-abi_so() {
-    local name path
-    name="$(abi_name)" || return 1
-    [ -n "$name" ] || return 1
-    path="$BUILD_DIR/bindings/python/rgbmatrix/$name"
-    [ -f "$path" ] || return 1
-    printf '%s\n' "$path"
-}
-
-do_rollback() {
-    local dst; dst="$(py_site)"
-    [ -n "$dst" ] || die "could not locate the installed rgbmatrix package"
-    [ -f "$BACKUP" ] || die "no backup at $BACKUP"
-    service_do stop
-    cp -a "$BACKUP" "$dst/core.so" || die "restore failed"
-    find "$dst" -name __pycache__ -type d -exec rm -rf {} + 2>/dev/null
-    service_do start
-    echo "rolled back to the original core.so"
-    exit 0
-}
-
-do_install() {
-    local so dst
-    so="$(abi_so)"; [ -n "$so" ] || die "no built module found - run the build first"
-    dst="$(py_site)"; [ -n "$dst" ] || die "could not locate the installed rgbmatrix package"
-
-    if [ ! -f "$BACKUP" ]; then
-        cp -a "$dst/core.so" "$BACKUP" || die "could not back up the original"
-        echo "backed up original core.so -> $BACKUP"
-    else
-        echo "backup already present at $BACKUP (keeping the true original)"
-    fi
-
-    service_do stop
-    cp "$so" "$dst/core.so" || die "install failed"
-    find "$dst" -name __pycache__ -type d -exec rm -rf {} + 2>/dev/null
-    service_do start
-
-    echo "waiting 25s for the display to come back..."
-    sleep 25
-    local healthy=1
-    systemctl is-active --quiet ledmatrix || healthy=0
-    if journalctl -u ledmatrix --since "40 sec ago" --no-pager \
-         | grep -qiE "Traceback|ImportError|Segmentation fault|undefined symbol"; then
-        healthy=0
-    fi
-    if [ "$healthy" = "1" ]; then
-        echo "SUCCESS - running on the rebuilt binding"
-    else
-        echo "UNHEALTHY - rolling back"
-        cp -a "$BACKUP" "$dst/core.so" \
-            || echo "ROLLBACK FAILED: could not restore $BACKUP -> $dst/core.so" >&2
-        if service_present && ! systemctl restart ledmatrix; then
-            echo "ROLLBACK FAILED: ledmatrix did not restart - the display is" \
-                 "down; restore manually with 'sudo bash $0 --rollback'" >&2
-        fi
-        journalctl -u ledmatrix --since "90 sec ago" --no-pager | tail -25
-        exit 1
-    fi
-    exit 0
-}
-
-case "${1:-}" in
-    --rollback) do_rollback ;;
-    --install)  do_install ;;
-    "" )        ;;
-    *)          die "unknown option: $1" ;;
-esac
-
-# ---------------------------------------------------------------- build ----
-[ -d "$SRC_TREE" ] || die "matrix source tree not found at $SRC_TREE (set RGB_SRC_TREE)"
-command -v g++ >/dev/null || die "g++ not installed (apt install build-essential)"
-
-echo "==> staging a scratch copy at $BUILD_DIR"
-rm -rf "$BUILD_DIR"
-cp -r "$SRC_TREE" "$BUILD_DIR" || die "copy failed"
-
-# Drop any extension artifacts that came across from the source tree. Nothing
-# downstream should be able to pick one up, and build_ext --inplace can decide
-# a copied .so is already up to date and skip the compile entirely.
-find "$BUILD_DIR/bindings/python/rgbmatrix" -maxdepth 1 \
-     -name 'core*.so' -delete 2>/dev/null
-
-echo "==> patching the bindings to release the GIL"
-python3 - "$BUILD_DIR" "$PATCH_BLIT" <<'PYEOF' || die "patch failed"
-import io
-import sys
-
-base = sys.argv[1] + "/bindings/python/rgbmatrix/"
-patch_blit = len(sys.argv) > 2 and sys.argv[2] == "1"
-
-# --- declare SwapOnVSync as nogil ---------------------------------------
-p = base + "cppinc.pxd"
-s = io.open(p, encoding="utf-8").read()
-OLD_DECL = "        FrameCanvas *SwapOnVSync(FrameCanvas*, uint8_t)\n"
-NEW_DECL = "        FrameCanvas *SwapOnVSync(FrameCanvas*, uint8_t) nogil\n"
-if OLD_DECL in s:
-    io.open(p, "w", encoding="utf-8", newline="\n").write(s.replace(OLD_DECL, NEW_DECL, 1))
-    print("   cppinc.pxd: SwapOnVSync declared nogil")
-elif NEW_DECL in s:
-    print("   cppinc.pxd: already nogil")
-else:
-    sys.exit("could not find the SwapOnVSync declaration")
-
-# --- release the GIL across the vsync wait ------------------------------
-p = base + "core.pyx"
-s = io.open(p, encoding="utf-8").read()
-
-OLD_SWAP = (
-    "    def SwapOnVSync(self, FrameCanvas newFrame, uint8_t framerate_fraction = 1):\n"
-    "        return __createFrameCanvas("
-    "self.__matrix.SwapOnVSync(newFrame.__canvas, framerate_fraction))\n"
-)
-NEW_SWAP = (
-    "    def SwapOnVSync(self, FrameCanvas newFrame, uint8_t framerate_fraction = 1):\n"
-    "        # Blocks until the panel's next vertical sync. Holding the GIL\n"
-    "        # across that wait starves every other Python thread for most of\n"
-    "        # each frame. Pointers are hoisted into C locals so the blocking\n"
-    "        # call itself needs no Python state.\n"
-    "        cdef cppinc.RGBMatrix* matrix = self.__matrix\n"
-    "        cdef cppinc.FrameCanvas* frame = newFrame.__canvas\n"
-    "        cdef uint8_t fraction = framerate_fraction\n"
-    "        cdef cppinc.FrameCanvas* swapped\n"
-    "        with nogil:\n"
-    "            swapped = matrix.SwapOnVSync(frame, fraction)\n"
-    "        return __createFrameCanvas(swapped)\n"
-)
-if OLD_SWAP in s:
-    s = s.replace(OLD_SWAP, NEW_SWAP, 1)
-    print("   core.pyx: SwapOnVSync releases the GIL")
-elif "swapped = matrix.SwapOnVSync(frame, fraction)" in s:
-    print("   core.pyx: SwapOnVSync already patched")
-else:
-    sys.exit("could not find the SwapOnVSync body")
-
-# --- optional: release the GIL across the blit --------------------------
-OLD_BLIT = (
-    "        buffer = get_pillow_buffer(image_capsule)\n"
-    "\n"
-    "        for col in range(max(0, -xstart), min(width, frame_width - xstart)):\n"
-    "            for row in range(max(0, -ystart), min(height, frame_height - ystart)):\n"
-    "                pixel = buffer[row][col]\n"
-    "                r = (pixel ) & 0xFF\n"
-    "                g = (pixel >> 8) & 0xFF\n"
-    "                b = (pixel >> 16) & 0xFF\n"
-    "                my_canvas.SetPixel(xstart+col, ystart+row, r, g, b)\n"
-)
-NEW_BLIT = (
-    "        buffer = get_pillow_buffer(image_capsule)\n"
-    "\n"
-    "        # Bounds hoisted so the blit needs no Python state and can run\n"
-    "        # without the GIL: it touches only a C buffer and a C++ canvas.\n"
-    "        # NOTE: row-major order makes a torn frame show as a horizontal\n"
-    "        # split across the panel's halves. See the header before enabling.\n"
-    "        cdef int col_start = max(0, -xstart)\n"
-    "        cdef int col_end = min(width, frame_width - xstart)\n"
-    "        cdef int row_start = max(0, -ystart)\n"
-    "        cdef int row_end = min(height, frame_height - ystart)\n"
-    "\n"
-    "        with nogil:\n"
-    "            for row in range(row_start, row_end):\n"
-    "                for col in range(col_start, col_end):\n"
-    "                    pixel = buffer[row][col]\n"
-    "                    r = (pixel ) & 0xFF\n"
-    "                    g = (pixel >> 8) & 0xFF\n"
-    "                    b = (pixel >> 16) & 0xFF\n"
-    "                    my_canvas.SetPixel(xstart+col, ystart+row, r, g, b)\n"
-)
-if patch_blit:
-    if OLD_BLIT in s:
-        s = s.replace(OLD_BLIT, NEW_BLIT, 1)
-        print("   core.pyx: pixel blit releases the GIL, row-major")
-    elif "for row in range(row_start, row_end):" in s:
-        print("   core.pyx: blit already patched")
-    else:
-        sys.exit("could not find the SetPixelsPillow loop")
-else:
-    print("   core.pyx: blit left unpatched (RGB_PATCH_BLIT=1 to enable)")
-
-io.open(p, "w", encoding="utf-8", newline="\n").write(s)
-PYEOF
-
-echo "==> building librgbmatrix.a (this takes a few minutes)"
-nice -n 10 make -C "$BUILD_DIR/lib" -j2 >/dev/null 2>&1 \
-    || die "library build failed - rerun 'make -C $BUILD_DIR/lib' to see why"
-[ -f "$BUILD_DIR/lib/librgbmatrix.a" ] || die "librgbmatrix.a was not produced"
-
-echo "==> preparing Cython"
-[ -d "$VENV" ] || python3 -m venv --system-site-packages "$VENV" || die "venv failed"
-"$VENV/bin/pip" install --quiet cython || die "cython install failed"
-
-cat > "$BUILD_DIR/bindings/python/setup.py" <<'EOF'
-from setuptools import setup, Extension
-from Cython.Build import cythonize
-
-core = Extension(
-    "rgbmatrix.core",
-    sources=["rgbmatrix/core.pyx", "rgbmatrix/shims/pillow.c"],
-    include_dirs=["../../include", "rgbmatrix/shims"],
-    extra_objects=["../../lib/librgbmatrix.a"],
-    language="c++",
-    extra_compile_args=["-O3", "-Wall", "-fno-exceptions", "-std=c++11"],
-    extra_link_args=["-lrt", "-lm", "-lpthread"],
-)
-
-setup(name="rgbmatrix",
-      ext_modules=cythonize([core], language_level="3str",
-                            compiler_directives={"binding": False}))
-EOF
-
-echo "==> compiling the extension"
-( cd "$BUILD_DIR/bindings/python" && "$VENV/bin/python" setup.py build_ext --inplace ) \
-    >/dev/null 2>&1 || die "extension build failed"
-
-SO="$(abi_so)" || true
-[ -n "$SO" ] || die "no .so produced - expected $(abi_name) in $BUILD_DIR/bindings/python/rgbmatrix"
-
-# Verify the GIL really is released before anyone installs this.
-EXPECTED=1; [ "$PATCH_BLIT" = "1" ] && EXPECTED=2
-PAIRS=$(grep -c "PyEval_SaveThread\|Py_UNBLOCK_THREADS" \
-        "$BUILD_DIR/bindings/python/rgbmatrix/core.cpp")
-[ "$PAIRS" -ge "$EXPECTED" ] \
-    || die "generated C++ has $PAIRS GIL-release sites, expected >= $EXPECTED"
-
-echo
-echo "BUILT: $SO"
-echo "       ($PAIRS GIL-release site(s) in the generated C++)"
-echo
-echo "Install with:   sudo bash $0 --install"
-echo "Roll back with: sudo bash $0 --rollback"

scripts/run_plugin_tests.py

@@ -6,9 +6,6 @@ Discovers and runs tests for LEDMatrix plugins.
 Supports both unittest and pytest.
 """
 
-import os
-import re
-import subprocess  # nosec B404 - list-form argv only, no shell  # nosemgrep
 import sys
 import argparse
 from pathlib import Path
@@ -71,79 +68,6 @@ def _find_tests_in_dir(directory: Path) -> list:
     return sorted(set(test_files))
 
 
-def _is_script_style(path) -> bool:
-    """True when a test file is a standalone script, not a pytest module.
-
-    Most plugin tests are written as `def main()` plus an `if __name__ ==
-    "__main__"` guard and signal through an exit code. pytest collects zero
-    items from those, so handing them to pytest printed "no tests ran" and this
-    runner reported success over work it had not done -- 151 of 248 files on a
-    fully populated rig.
-    """
-    try:
-        src = Path(path).read_text(encoding="utf-8", errors="replace")
-    except OSError:
-        return False
-    has_pytest_items = re.search(r"^\s*(def test_|class Test|async def test_)", src, re.M)
-    has_main_guard = "__main__" in src and "__name__" in src
-    return bool(has_main_guard and not has_pytest_items)
-
-
-def run_script_tests(test_files: list, verbose: bool = False) -> int:
-    """Run standalone test scripts, honouring the 0 pass / 2 skip / 1 fail
-    convention that ledmatrix-plugins' own runner established.
-
-    Scripts opt into skipping by printing "SKIP: <reason>" and exiting 2 --
-    a script that needs a tty or an LED matrix is not a regression.
-    """
-    env = dict(os.environ)
-    # Prepend rather than setdefault. An inherited PYTHONPATH -- a developer's
-    # shell, a tox run, another checkout -- otherwise wins outright, and the
-    # subprocess imports a different copy of the core than the one under test.
-    # That is exactly the failure ledmatrix-plugins#467 describes, and it is
-    # invisible: the tests pass or fail against a tree nobody meant to test.
-    inherited = env.get("PYTHONPATH")
-    env["PYTHONPATH"] = (f"{PROJECT_ROOT}{os.pathsep}{inherited}"
-                         if inherited else str(PROJECT_ROOT))
-    env["LEDMATRIX_CORE"] = str(PROJECT_ROOT)
-
-    passed = skipped = failed = 0
-    failures = []
-    for path in test_files:
-        try:
-            # Fixed interpreter (sys.executable) plus a test path this script
-            # discovered by globbing the repo; argument list, no shell, so
-            # nothing is word-split or expanded. Same suppression pair the
-            # rest of the repo uses for this shape (see permission_utils.py).
-            proc = subprocess.run(  # noqa: S603  # nosec B603 - no shell invoked (list-form argv)  # nosemgrep
-                [sys.executable, str(path)],  # nosemgrep
-                cwd=str(Path(path).parent),
-                capture_output=True, text=True, env=env,
-                stdin=subprocess.DEVNULL, timeout=300,
-            )
-            rc = proc.returncode
-            tail = " | ".join((proc.stdout or proc.stderr or "").strip().splitlines()[-2:])[:200]
-        except subprocess.TimeoutExpired:
-            rc, tail = 1, "timed out after 300s"
-        if rc == 0:
-            passed += 1
-            label = "pass"
-        elif rc == 2:
-            skipped += 1
-            label = "SKIP"
-        else:
-            failed += 1
-            label = "FAIL"
-            failures.append(f"{Path(path).name}: exit {rc} | {tail}")
-        if verbose or rc != 0:
-            print(f"  [{label}] {Path(path).name}" + (f" -- {tail}" if rc != 0 else ""))
-
-    print(f"\n{passed} passed, {skipped} skipped, {failed} failed (scripts)")
-    for f in failures:
-        print(f"  - {f}", file=sys.stderr)
-    return 1 if failed else 0
-
-
 def run_unittest_tests(test_files: list, verbose: bool = False) -> int:
     """
     Run tests using unittest.
@@ -262,12 +186,7 @@ def main():
             print("No test files found in plugins directory")
         return 0
     
-    scripts = [f for f in test_files if _is_script_style(f)]
-    modules = [f for f in test_files if f not in scripts]
-
-    print(f"Found {len(test_files)} test file(s)"
-          + (f" -- {len(modules)} collectable, {len(scripts)} standalone script(s)"
-             if scripts else ""))
+    print(f"Found {len(test_files)} test file(s)")
     for test_file in test_files:
         print(f"  - {test_file}")
     print()
@@ -281,20 +200,11 @@ def main():
         except ImportError:
             runner = 'unittest'
     
-    # Standalone scripts cannot be collected by pytest or unittest -- run them
-    # as the scripts they are. Doing this rather than silently collecting zero
-    # items is the whole point: this runner used to report success having
-    # executed nothing.
-    rc = 0
-    if scripts:
-        rc |= run_script_tests(scripts, args.verbose)
-
-    if modules:
+    # Run tests
     if runner == 'pytest':
-            rc |= run_pytest_tests(modules, args.verbose, args.coverage)
+        return run_pytest_tests(test_files, args.verbose, args.coverage)
     else:
-            rc |= run_unittest_tests(modules, args.verbose)
-    return rc
+        return run_unittest_tests(test_files, args.verbose)
 
 
 if __name__ == '__main__':

scripts/scroll_speeds.py

@@ -1,238 +0,0 @@
-#!/usr/bin/env python3
-"""Show and try the scroll speeds your panel can display cleanly.
-
-Motion looks smooth when the strip advances a WHOLE number of pixels per panel
-refresh. Anything else has to blend two columns (which on pixel-font text reads
-as shimmer) or repeat frames unevenly (which reads as judder). So the speeds
-worth using are not arbitrary -- they are
-
-    refresh_hz / frame_hold * pixels_per_frame
-
-for whole numbers of frame_hold and pixels_per_frame, and that ladder depends
-on how fast YOUR panel actually refreshes. A Pi Zero driving a big chain will
-have a completely different set of good speeds from a Pi 4 driving a small one.
-
-    # what can this panel do? (no hardware needed, uses your configured rate)
-    python3 scripts/scroll_speeds.py
-
-    # measure what the panel ACTUALLY manages, rather than what is configured
-    sudo systemctl stop ledmatrix
-    sudo python3 scripts/scroll_speeds.py --measure
-    sudo systemctl start ledmatrix
-
-    # what would a 60Hz panel offer?
-    python3 scripts/scroll_speeds.py --hz 60
-
-    # try one on the panel
-    sudo systemctl stop ledmatrix
-    sudo python3 scripts/scroll_speeds.py --demo 50
-    sudo systemctl start ledmatrix
-
-This script never starts or stops the display service itself -- that is left to
-you, so a crash here can never leave the panel dark.
-"""
-from __future__ import annotations
-
-import argparse
-import json
-import os
-import sys
-import time
-from pathlib import Path
-
-sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
-
-from src.common import scroll_config  # noqa: E402
-
-CONFIG = Path(__file__).resolve().parent.parent / "config" / "config.json"
-
-
-def load_hardware():
-    try:
-        with open(CONFIG, encoding="utf-8") as handle:
-            return (json.load(handle).get("display") or {}).get("hardware") or {}
-    except (OSError, ValueError):
-        return {}
-
-
-def build_options(hardware, refresh_override=None):
-    from rgbmatrix import RGBMatrixOptions
-
-    o = RGBMatrixOptions()
-    o.rows = int(hardware.get("rows", 32))
-    o.cols = int(hardware.get("cols", 64))
-    o.chain_length = int(hardware.get("chain_length", 1))
-    o.parallel = int(hardware.get("parallel", 1))
-    o.brightness = int(hardware.get("brightness", 80))
-    o.hardware_mapping = hardware.get("hardware_mapping", "regular")
-    o.pwm_bits = int(hardware.get("pwm_bits", 11))
-    o.pwm_dither_bits = int(hardware.get("pwm_dither_bits", 0))
-    o.pwm_lsb_nanoseconds = int(hardware.get("pwm_lsb_nanoseconds", 130))
-    o.led_rgb_sequence = hardware.get("led_rgb_sequence", "RGB")
-    o.scan_mode = int(hardware.get("scan_mode", 0))
-    o.row_address_type = int(hardware.get("row_address_type", 0))
-    o.multiplexing = int(hardware.get("multiplexing", 0))
-    o.gpio_slowdown = int(hardware.get("gpio_slowdown", 2))
-    o.limit_refresh_rate_hz = (
-        int(refresh_override) if refresh_override is not None
-        else int(hardware.get("limit_refresh_rate_hz", 0))
-    )
-    return o
-
-
-def open_matrix(hardware, refresh_override=None):
-    """Construct the matrix, or explain why it will not open."""
-    if os.geteuid() != 0:
-        sys.exit("this needs root for GPIO access - rerun with sudo")
-    try:
-        from rgbmatrix import RGBMatrix
-    except ImportError:
-        sys.exit("rgbmatrix is not installed on this machine")
-    try:
-        return RGBMatrix(options=build_options(hardware, refresh_override))
-    except Exception as exc:  # pragma: no cover - hardware dependent
-        sys.exit(
-            "could not open the panel ({}).\n"
-            "If the display service is running it owns the GPIO - stop it first:\n"
-            "    sudo systemctl stop ledmatrix".format(exc)
-        )
-
-
-def measure_refresh(hardware, seconds=6.0):
-    """Actual refresh rate, by running uncapped and timing the swaps.
-
-    SwapOnVSync blocks until the panel's next refresh, so an unthrottled loop
-    runs at exactly the panel's rate. This is what an older Pi or a longer
-    chain will really give you, as opposed to whatever limit_refresh_rate_hz
-    optimistically asks for.
-    """
-    matrix = open_matrix(hardware, refresh_override=0)
-    canvas = matrix.CreateFrameCanvas()
-    canvas = matrix.SwapOnVSync(canvas)  # discard the first, it includes setup
-    frames = 0
-    started = time.perf_counter()
-    while time.perf_counter() - started < seconds:
-        canvas = matrix.SwapOnVSync(canvas)
-        frames += 1
-    measured = frames / (time.perf_counter() - started)
-    matrix.Clear()
-    return measured
-
-
-def demo(hardware, target, seconds):
-    """Scroll text at the crisp speed nearest `target`."""
-    from PIL import Image, ImageDraw, ImageFont
-
-    hz = float(hardware.get("limit_refresh_rate_hz") or scroll_config.DEFAULT_REFRESH_HZ)
-    choice = scroll_config.solve_crisp(target, hz)
-    print("asked for {:.0f} px/s -> {}".format(target, choice.describe()))
-
-    matrix = open_matrix(hardware)
-    canvas = matrix.CreateFrameCanvas()
-    W, H = canvas.width, canvas.height
-
-    font = None
-    for path, size in (
-        (str(Path(__file__).resolve().parent.parent / "assets/fonts/PressStart2P-Regular.ttf"), 16),
-        ("/usr/share/fonts/truetype/dejavu/DejaVuSansMono-Bold.ttf", 26),
-    ):
-        try:
-            font = ImageFont.truetype(path, size)
-            break
-        except OSError:
-            continue
-    if font is None:
-        font = ImageFont.load_default()
-
-    text = "  {:.0f} px/s  ***  THE QUICK BROWN FOX JUMPS OVER THE LAZY DOG  ***".format(
-        choice.pixels_per_second)
-    box = ImageDraw.Draw(Image.new("RGB", (8, 8))).textbbox((0, 0), text, font=font)
-    tw, th = box[2] - box[0], box[3] - box[1]
-    reps = max(2, (W * 3) // max(tw, 1) + 1)
-    strip = Image.new("RGB", (tw * reps, H), (0, 0, 0))
-    draw = ImageDraw.Draw(strip)
-    for i in range(reps):
-        draw.text((i * tw, (H - th) // 2 - box[1]), text, font=font, fill=(255, 210, 60))
-
-    offset = 0
-    frames = 0
-    started = time.time()
-    while time.time() - started < seconds:
-        window = strip.crop((offset, 0, offset + W, H))
-        if window.width < W:
-            whole = Image.new("RGB", (W, H), (0, 0, 0))
-            head = strip.crop((offset, 0, strip.width, H))
-            whole.paste(head, (0, 0))
-            whole.paste(strip.crop((0, 0, W - head.width, H)), (head.width, 0))
-            window = whole
-        canvas.SetImage(window)
-        canvas = matrix.SwapOnVSync(canvas, choice.frame_hold)
-        offset = (offset + choice.pixels_per_frame) % strip.width
-        frames += 1
-    elapsed = time.time() - started
-    print("  {} frames in {:.1f}s = {:.1f} fps = {:.1f} px/s actual".format(
-        frames, elapsed, frames / elapsed, frames * choice.pixels_per_frame / elapsed))
-    matrix.Clear()
-
-
-def print_ladder(hz, highlight=None):
-    print("")
-    print("Whole-pixel scroll speeds at {:.1f}Hz refresh".format(hz))
-    print("(the panel refreshes at {:.0f}Hz for every one of these - holding a "
-          "frame costs no flicker)".format(hz))
-    print("")
-    for entry in scroll_config.crisp_ladder(hz):
-        if entry.pixels_per_second > hz * 3:
-            break
-        mark = " <-- nearest to {:.0f}".format(highlight) if (
-            highlight is not None
-            and entry.pixels_per_second == scroll_config.solve_crisp(highlight, hz).pixels_per_second
-        ) else ""
-        print("  " + entry.describe() + mark)
-    print("")
-    print("Set one in config.json as pixels per second, e.g.")
-    print('  "display_options": {{"scroll_pixels_per_second": {:.0f}}}'.format(
-        scroll_config.solve_crisp(highlight if highlight else hz / 2, hz).pixels_per_second))
-
-
-def main():
-    ap = argparse.ArgumentParser(description=__doc__,
-                                 formatter_class=argparse.RawDescriptionHelpFormatter)
-    ap.add_argument("--hz", type=float,
-                    help="refresh rate to compute the ladder for (default: your config)")
-    ap.add_argument("--measure", action="store_true",
-                    help="measure the panel's real refresh rate (needs root, service stopped)")
-    ap.add_argument("--demo", type=float, metavar="PXPS",
-                    help="scroll text at the crisp speed nearest this (needs root)")
-    ap.add_argument("--seconds", type=float, default=15.0, help="demo duration")
-    ap.add_argument("--want", type=float, metavar="PXPS",
-                    help="highlight the entry nearest this speed")
-    args = ap.parse_args()
-
-    hardware = load_hardware()
-    configured = float(hardware.get("limit_refresh_rate_hz") or 0)
-
-    if args.demo is not None:
-        demo(hardware, args.demo, args.seconds)
-        return
-
-    if args.measure:
-        measured = measure_refresh(hardware)
-        print("measured panel refresh: {:.1f}Hz".format(measured))
-        if configured:
-            print("configured limit_refresh_rate_hz: {:.0f}".format(configured))
-            if measured < configured * 0.95:
-                print("  -> the panel cannot reach the configured rate; the ladder")
-                print("     below uses what it actually manages")
-        print_ladder(measured, args.want)
-        return
-
-    hz = args.hz or configured or scroll_config.DEFAULT_REFRESH_HZ
-    if not args.hz and not configured:
-        print("no limit_refresh_rate_hz in config; assuming {:.0f}Hz".format(hz))
-        print("run with --measure to find your panel's real rate")
-    print_ladder(hz, args.want)
-
-
-if __name__ == "__main__":
-    main()

src/cache/disk_cache.py

@@ -5,7 +5,6 @@ Handles persistent disk-based caching with atomic writes and error recovery.
 """
 
 import json
-import math
 import os
 import time
 import tempfile
@@ -15,11 +14,6 @@ import zlib
 from typing import Dict, Any, Optional, Protocol
 from datetime import datetime
 
-try:  # optional: large speedup on the cache write path, see _dumps below
-    import orjson
-except ImportError:  # pragma: no cover - exercised on hosts without the wheel
-    orjson = None
-
 # How old an abandoned write's temp file must be before the sweep removes it.
 # A real write holds its temp file for milliseconds, so an hour is far beyond
 # any in-flight write while still clearing the same day's debris. Deliberately
@@ -46,97 +40,13 @@ class CacheStrategyProtocol(Protocol):
 
 
 class DateTimeEncoder(json.JSONEncoder):
-    """JSON encoder that handles datetime objects.
-
-    Retained for the stdlib fallback path and for any caller importing it.
-    """
+    """JSON encoder that handles datetime objects."""
     def default(self, obj: Any) -> Any:
         if isinstance(obj, datetime):
             return obj.isoformat()
         return super().default(obj)
 
 
-def _datetime_default(obj: Any) -> Any:
-    """Serialise datetimes exactly as DateTimeEncoder did."""
-    if isinstance(obj, datetime):
-        return obj.isoformat()
-    raise TypeError(f"Object of type {type(obj).__name__} is not JSON serializable")
-
-
-def _replace_nonfinite(obj: Any) -> Any:
-    """Non-finite floats -> None, matching what ``orjson.dumps`` writes.
-
-    Only reached once a strict pass has proved there is something to replace,
-    so the ordinary write path never pays for this walk.
-    """
-    if isinstance(obj, float):
-        return obj if math.isfinite(obj) else None
-    if isinstance(obj, dict):
-        return {k: _replace_nonfinite(v) for k, v in obj.items()}
-    if isinstance(obj, (list, tuple)):
-        return [_replace_nonfinite(v) for v in obj]
-    return obj
-
-
-# NON-FINITE FLOATS
-# -----------------
-# JSON has no NaN or Infinity. The stdlib emits them anyway as an extension;
-# orjson refuses to and writes null. That divergence is not acceptable in a
-# cache whose files outlive the decision of which encoder is installed, so the
-# policy here is one behaviour on both paths:
-#
-#   writing  non-finite floats become null, whichever encoder is in use
-#   reading  files already on disk that carry the stdlib's NaN/Infinity
-#            tokens stay readable, whichever encoder is in use
-#
-# Without the write half, installing orjson silently changed cached values.
-# Without the read half, installing orjson turned every legacy record holding a
-# NaN into a "corrupted cache file" that DiskCache.get logged as an error and
-# deleted. Both halves are covered by test/test_cache_nonfinite_floats.py.
-
-
-if orjson is not None:
-    # Encoding the cache record dominated the background fetch worker: on a
-    # Pi 4, stdlib json.dumps runs ~12ms per MB and holds the GIL for all of
-    # it, which stalls the render thread mid-scroll. orjson measures ~7x
-    # faster on the same payloads (11.9ms -> 1.6ms for 985KB). Decoding gains
-    # far less (~1.3x on large payloads) because the cost there is building
-    # the Python objects, not scanning the text, but it is still free to take.
-    #
-    # OPT_NON_STR_KEYS: stdlib json coerces int/float dict keys to strings;
-    # orjson raises without this, and cache records do carry numeric keys.
-    # OPT_PASSTHROUGH_DATETIME: orjson would otherwise emit its own RFC 3339
-    # form for datetimes instead of calling default(). Routing them through
-    # _datetime_default keeps byte-for-byte parity with the records already
-    # on disk.
-    _DUMPS_OPTS = orjson.OPT_NON_STR_KEYS | orjson.OPT_PASSTHROUGH_DATETIME
-
-    def _dumps(data: Any) -> bytes:
-        return orjson.dumps(data, default=_datetime_default, option=_DUMPS_OPTS)
-
-    def _loads(raw: bytes) -> Any:
-        try:
-            return orjson.loads(raw)
-        except orjson.JSONDecodeError:
-            # Legacy record written by the stdlib path, carrying NaN or
-            # Infinity. Genuinely malformed files raise again from here, as
-            # json.JSONDecodeError, which is what DiskCache.get expects.
-            return json.loads(raw)
-else:
-    def _dumps(data: Any) -> bytes:
-        try:
-            return json.dumps(data, cls=DateTimeEncoder,
-                              allow_nan=False).encode("utf-8")
-        except ValueError:
-            # allow_nan=False is what detects the non-finite values; the walk
-            # runs only now that we know there is one to replace.
-            return json.dumps(_replace_nonfinite(data), cls=DateTimeEncoder,
-                              allow_nan=False).encode("utf-8")
-
-    def _loads(raw: bytes) -> Any:
-        return json.loads(raw)
-
-
 class DiskCache:
     """Manages persistent disk-based cache."""
     
@@ -189,8 +99,8 @@ class DiskCache:
         
         try:
             with self._lock:
-                with open(cache_path, 'rb') as f:
-                    record = _loads(f.read())
+                with open(cache_path, 'r', encoding='utf-8') as f:
+                    record = json.load(f)
             
             # Determine record timestamp (prefer embedded, else file mtime)
             record_ts = None
@@ -279,12 +189,12 @@ class DiskCache:
         # write path below, and cache files are machine-read only — indenting
         # them just multiplied the bytes written to the SD card.
         try:
-            payload = _dumps(data)
+            payload = json.dumps(data, cls=DateTimeEncoder)
         except (TypeError, ValueError) as e:
             self.logger.warning("Cache data for key '%s' not serializable: %s", key, e)
             return
 
-        digest = zlib.adler32(payload)
+        digest = zlib.adler32(payload.encode('utf-8'))
 
         try:
             # Atomic write to avoid partial/corrupt files
@@ -332,7 +242,7 @@ class DiskCache:
                         # wear source (dozens of fsyncs/min on API-heavy
                         # installs) for data that can be re-downloaded.
                         try:
-                            with os.fdopen(fd, 'wb') as tmp_file:
+                            with os.fdopen(fd, 'w', encoding='utf-8') as tmp_file:
                                 tmp_file.write(payload)
                             os.replace(tmp_path, cache_path)
                             self._write_digests[key] = digest
@@ -350,7 +260,7 @@ class DiskCache:
                     else:
                         # Fallback: direct write (not atomic, but better than failing)
                         try:
-                            with open(cache_path, 'wb') as cache_file:
+                            with open(cache_path, 'w', encoding='utf-8') as cache_file:
                                 cache_file.write(payload)
                             self._write_digests[key] = digest
                             # Set proper permissions: 660 (rw-rw----) for group-readable cache files
@@ -380,7 +290,7 @@ class DiskCache:
                             # is a different path, so future sets must keep
                             # retrying the primary location.
                             fallback_path = os.path.join(fallback_dir, os.path.basename(cache_path))
-                            with open(fallback_path, 'wb') as tmp_file:
+                            with open(fallback_path, 'w', encoding='utf-8') as tmp_file:
                                 tmp_file.write(payload)
                             # Set proper permissions: 660 (rw-rw----) for group-readable cache files
                             try:

src/common/__init__.py

@@ -23,13 +23,6 @@ from src.common.error_handler import (
 )
 from src.common.api_helper import APIHelper
 from src.common.scroll_helper import ScrollHelper
-from src.common import scroll_config
-from src.common.scroll_config import (
-    ScrollSettings,
-    configure as configure_scroll,
-    resolve as resolve_scroll_settings,
-    refresh_hz_from_config,
-)
 from src.common.logo_helper import LogoHelper
 from src.common.text_helper import TextHelper
 
@@ -67,11 +60,6 @@ __all__ = [
     'log_and_raise',
     'APIHelper',
     'ScrollHelper',
-    'scroll_config',
-    'ScrollSettings',
-    'configure_scroll',
-    'resolve_scroll_settings',
-    'refresh_hz_from_config',
     'LogoHelper',
     'TextHelper',
     # adaptive layout & images

src/common/scroll_config.py

@@ -1,440 +0,0 @@
-"""One place that turns plugin config into a configured ScrollHelper.
-
-Five ticker plugins each hand-rolled this resolution (odds-ticker, news and
-ledmatrix-leaderboard reference the deprecated ``scroll_pixels_per_second``
-key 16-18 times apiece), and they disagreed in ways that were invisible until
-someone watched the panel:
-
-* odds-ticker read ``scroll_pixels_per_second`` on the *recommended* config
-  path and let it override ``scroll_speed``/``scroll_delay``. Because that key
-  carries a schema default, the documented settings were dead for every user
-  -- see ChuckBuilds/ledmatrix-plugins#408.
-* ledmatrix-leaderboard read the same key only as a fallback, so identical
-  config produced different speeds in the two plugins.
-* stock-news derived px/frame from it via its own arithmetic.
-
-What matters on the hardware
-----------------------------
-Motion is smooth when the strip advances a **whole number of pixels per panel
-refresh**. On a 100Hz panel that means 100 px/s, 200 px/s, and so on. Anything
-else has to either blend adjacent columns (which on pixel-font text reads as
-shimmer) or repeat frames (which reads as judder). :func:`resolve` warns when
-the requested speed will not divide evenly, because that is a real display
-artefact and not a rounding detail.
-
-Speed is always expressed to the helper as pixels per second and applied in
-time-based mode. Frame-based stepping gates motion on a wall clock at
-``1/scroll_delay`` steps per second; plugins set ``scroll_delay`` to the frame
-period, which puts that comparison exactly on its own threshold and makes the
-step count flip on sub-millisecond jitter. Accumulating elapsed time keeps
-position proportional to real time instead.
-"""
-
-from __future__ import annotations
-
-import logging
-from dataclasses import dataclass, replace
-from typing import Any, Dict, Optional
-
-logger = logging.getLogger(__name__)
-
-#: Speed used when a plugin supplies nothing usable. One pixel per refresh on a
-#: 100Hz panel, which is the slowest crisp scroll that hardware can show.
-DEFAULT_PIXELS_PER_SECOND = 100.0
-
-#: Bounds accepted from config. Below the floor a marquee appears frozen;
-#: above the ceiling it outruns any panel's refresh and tears.
-MIN_PIXELS_PER_SECOND = 1.0
-MAX_PIXELS_PER_SECOND = 500.0
-
-#: Assumed refresh when the caller does not say. Matches the usual
-#: ``display.hardware.limit_refresh_rate_hz``.
-DEFAULT_REFRESH_HZ = 100.0
-
-#: How far px/s may sit from a whole number of pixels per refresh before it is
-#: worth warning about. 0.05px per frame is invisible; a third of a pixel is not.
-_WHOLE_PIXEL_TOLERANCE = 0.05
-
-
-#: Longest a frame may be held before motion reads as a slideshow rather than
-#: a scroll. 6 refreshes at 100Hz is ~17px/s, already visibly stepped.
-MAX_FRAME_HOLD = 8
-
-#: Largest whole-pixel jump per presented frame before motion looks like it is
-#: teleporting rather than sliding.
-MAX_PIXELS_PER_FRAME = 6
-
-
-@dataclass(frozen=True)
-class CrispSpeed:
-    """A speed the panel can show with whole-pixel motion.
-
-    ``pixels_per_second`` is always ``refresh_hz / frame_hold * pixels_per_frame``
-    exactly -- no rounding, no fractional pixel positions, so nothing has to be
-    blended or repeated unevenly.
-
-    :param frame_hold: refreshes each frame is held for. This is rgbmatrix's
-        ``SwapOnVSync(canvas, framerate_fraction)``. The panel keeps refreshing
-        at full rate either way, so holding a frame costs nothing in flicker.
-    :param pixels_per_frame: whole pixels advanced per presented frame.
-    """
-
-    pixels_per_second: float
-    frame_hold: int
-    pixels_per_frame: int
-    refresh_hz: float
-
-    @property
-    def frames_per_second(self) -> float:
-        return self.refresh_hz / self.frame_hold
-
-    @property
-    def steppiness(self) -> str:
-        """Rough readability hint for this combination."""
-        if self.pixels_per_frame > 2:
-            return "jumpy"
-        if self.frames_per_second < 20:
-            return "stepped"
-        if self.frames_per_second < 30:
-            return "slightly stepped"
-        return "smooth"
-
-    def describe(self) -> str:
-        return (
-            f"{self.pixels_per_second:6.1f} px/s  "
-            f"({self.pixels_per_frame}px every {self.frame_hold} refresh"
-            f"{'es' if self.frame_hold != 1 else ' '} = "
-            f"{self.frames_per_second:5.1f} fps, {self.steppiness})"
-        )
-
-
-def crisp_ladder(
-    refresh_hz: float = DEFAULT_REFRESH_HZ,
-    max_frame_hold: int = MAX_FRAME_HOLD,
-    max_pixels_per_frame: int = MAX_PIXELS_PER_FRAME,
-):
-    """Every whole-pixel speed this panel can show, slowest first.
-
-    Duplicates are collapsed keeping the gentlest option: 100 px/s is reachable
-    as 1px every refresh or 2px every 2nd refresh, and the former moves in
-    smaller increments, so that is the one worth offering.
-    """
-    best = {}
-    for hold in range(1, max_frame_hold + 1):
-        for ppf in range(1, max_pixels_per_frame + 1):
-            pps = refresh_hz / hold * ppf
-            key = round(pps, 3)
-            candidate = CrispSpeed(pps, hold, ppf, refresh_hz)
-            incumbent = best.get(key)
-            if incumbent is None or ppf < incumbent.pixels_per_frame:
-                best[key] = candidate
-    return [best[k] for k in sorted(best)]
-
-
-#: How much a bigger pixel step costs, as a fraction of the target speed.
-#: Tuned so 66.7px/s (2px at 33fps) beats 50px/s (1px at 50fps) when 60 was
-#: asked for, but 33.3px/s (1px, smooth) still beats 28.6px/s (2px at 14fps)
-#: when 30 was asked for -- being 11% slow is worth far less than looking bad.
-_STEP_PENALTY = 0.05
-_SLOW_FPS_PENALTY = 0.25   # below 20fps
-_LOWISH_FPS_PENALTY = 0.10  # below 25fps
-
-
-def _quality_cost(candidate: "CrispSpeed", target: float) -> float:
-    """Lower is better. Numeric closeness alone picks bad-looking speeds.
-
-    Nearest-by-value would answer "30 px/s" with 28.6 px/s -- which is 2px
-    jumps at 14fps -- over 33.3 px/s, which is single-pixel motion at 33fps and
-    obviously better on the panel. Proximity has to be traded against how the
-    motion actually reads.
-    """
-    error = abs(candidate.pixels_per_second - target) / max(target, 1e-6)
-    cost = error + _STEP_PENALTY * (candidate.pixels_per_frame - 1)
-    fps = candidate.frames_per_second
-    if fps < 20:
-        cost += _SLOW_FPS_PENALTY
-    elif fps < 25:
-        cost += _LOWISH_FPS_PENALTY
-    return cost
-
-
-def solve_crisp(
-    target_pixels_per_second: float,
-    refresh_hz: float = DEFAULT_REFRESH_HZ,
-    max_frame_hold: int = MAX_FRAME_HOLD,
-    max_pixels_per_frame: int = MAX_PIXELS_PER_FRAME,
-) -> CrispSpeed:
-    """The whole-pixel speed that will look best for what was asked for.
-
-    Not simply the nearest -- see :func:`_quality_cost`. Ties break toward the
-    smaller pixel step and the shorter hold.
-    """
-    ladder = crisp_ladder(refresh_hz, max_frame_hold, max_pixels_per_frame)
-    # Clamp into the ladder's range first. Relative error saturates near 1.0
-    # for a target far outside it, so the quality penalty would dominate and
-    # answer "10000 px/s" with the *slowest* entry -- smooth, and useless.
-    target = min(max(target_pixels_per_second, ladder[0].pixels_per_second),
-                 ladder[-1].pixels_per_second)
-    return min(
-        ladder,
-        key=lambda c: (round(_quality_cost(c, target), 6),
-                       c.pixels_per_frame, c.frame_hold),
-    )
-
-
-@dataclass(frozen=True)
-class ScrollSettings:
-    """The resolved outcome, and which config key produced it."""
-
-    pixels_per_second: float
-    source: str
-    target_fps: Optional[float] = None
-    pixels_per_frame: Optional[float] = None
-    warning: Optional[str] = None
-    #: The whole-pixel speed actually applied, when snapping was enabled.
-    crisp: Optional[CrispSpeed] = None
-    #: What the config asked for, before snapping.
-    requested_pixels_per_second: Optional[float] = None
-
-    @property
-    def frame_hold(self) -> int:
-        """Refreshes to hold each frame for; pass to set_scrolling_state()."""
-        return self.crisp.frame_hold if self.crisp else 1
-
-    def describe(self) -> str:
-        text = f"{self.pixels_per_second:.1f} px/s (from {self.source})"
-        if self.pixels_per_frame is not None:
-            text += f" = {self.pixels_per_frame:.2f} px/frame"
-        if self.target_fps:
-            text += f" at {self.target_fps:.0f} fps"
-        return text
-
-
-def _coerce(value: Any) -> Optional[float]:
-    """A positive float, or None. Config reaches us with nulls and strings."""
-    if value is None or isinstance(value, bool):
-        return None
-    try:
-        number = float(value)
-    except (TypeError, ValueError):
-        return None
-    return number if number > 0 else None
-
-
-def _from_speed_and_delay(block: Any) -> Optional[float]:
-    """px/s from a ``scroll_speed`` (px/frame) + ``scroll_delay`` (s) pair."""
-    if not isinstance(block, dict):
-        return None
-    speed = _coerce(block.get("scroll_speed"))
-    delay = _coerce(block.get("scroll_delay"))
-    if speed is None or delay is None:
-        return None
-    return speed / delay
-
-
-def resolve(
-    plugin_config: Optional[Dict[str, Any]] = None,
-    global_config: Optional[Dict[str, Any]] = None,
-    default_pixels_per_second: float = DEFAULT_PIXELS_PER_SECOND,
-    refresh_hz: Optional[float] = None,
-) -> ScrollSettings:
-    """Resolve one scroll speed from the several shapes plugins accept.
-
-    Precedence, highest first. The deprecated flat key sits *below* the
-    explicit pairs deliberately: it carries schema defaults in some plugins, so
-    ranking it above them silently disables the documented settings.
-
-    1. ``display_options.scroll_speed`` + ``scroll_delay``  (current)
-    2. ``display.scroll_speed`` + ``scroll_delay``          (deprecated shape)
-    3. ``scroll_speed`` + ``scroll_delay`` at the root      (legacy flat)
-    4. ``scroll_pixels_per_second``, nested or flat         (deprecated)
-    5. the global ``display`` block
-    6. ``default_pixels_per_second``
-
-    :param refresh_hz: panel refresh, used only to check whether the resolved
-        speed lands on whole pixels per frame and to fill in ``target_fps``.
-    """
-    plugin_config = plugin_config or {}
-    global_config = global_config or {}
-    refresh = _coerce(refresh_hz) or DEFAULT_REFRESH_HZ
-
-    display_options = plugin_config.get("display_options")
-    display_block = plugin_config.get("display")
-
-    candidates = [
-        (_from_speed_and_delay(display_options), "display_options.scroll_speed/delay"),
-        (_from_speed_and_delay(display_block), "display.scroll_speed/delay"),
-        (_from_speed_and_delay(plugin_config), "scroll_speed/delay (root)"),
-    ]
-    for block, label in (
-        (display_options, "display_options.scroll_pixels_per_second"),
-        (display_block, "display.scroll_pixels_per_second"),
-        (plugin_config, "scroll_pixels_per_second"),
-    ):
-        if isinstance(block, dict):
-            candidates.append((_coerce(block.get("scroll_pixels_per_second")), label))
-
-    global_display = global_config.get("display")
-    candidates.append((_from_speed_and_delay(global_display), "global display.scroll_speed/delay"))
-
-    pixels_per_second = None
-    source = "default"
-    for value, label in candidates:
-        if value is not None:
-            pixels_per_second, source = value, label
-            break
-    if pixels_per_second is None:
-        pixels_per_second = default_pixels_per_second
-
-    clamped = max(MIN_PIXELS_PER_SECOND, min(MAX_PIXELS_PER_SECOND, pixels_per_second))
-    warning = None
-    if clamped != pixels_per_second:
-        warning = (
-            f"scroll speed {pixels_per_second:.1f} px/s out of range, "
-            f"clamped to {clamped:.1f}"
-        )
-        pixels_per_second = clamped
-
-    pixels_per_frame = pixels_per_second / refresh if refresh > 0 else None
-    if warning is None and pixels_per_frame is not None:
-        offset = abs(pixels_per_frame - round(pixels_per_frame))
-        if pixels_per_frame < 1.0 - _WHOLE_PIXEL_TOLERANCE or offset > _WHOLE_PIXEL_TOLERANCE:
-            suggestion = max(1.0, round(pixels_per_frame)) * refresh
-            warning = (
-                f"{pixels_per_second:.1f} px/s is {pixels_per_frame:.2f} px per "
-                f"refresh at {refresh:.0f}Hz, so some frames repeat and the "
-                f"scroll will judder; {suggestion:.0f} px/s divides evenly"
-            )
-
-    return ScrollSettings(
-        pixels_per_second=pixels_per_second,
-        source=source,
-        target_fps=refresh,
-        pixels_per_frame=pixels_per_frame,
-        warning=warning,
-    )
-
-
-def configure(
-    scroll_helper: Any,
-    plugin_config: Optional[Dict[str, Any]] = None,
-    global_config: Optional[Dict[str, Any]] = None,
-    default_pixels_per_second: float = DEFAULT_PIXELS_PER_SECOND,
-    refresh_hz: Optional[float] = None,
-    plugin_logger: Optional[logging.Logger] = None,
-    display_manager: Any = None,
-    snap_to_crisp: bool = True,
-) -> ScrollSettings:
-    """Resolve the config and apply it to ``scroll_helper``.
-
-    Applied in time-based mode: see the module docstring for why frame-based
-    stepping is not used. ``hasattr`` guards keep this usable against older
-    ScrollHelper builds that a plugin may be running on.
-
-    :param display_manager: consulted for the panel's refresh rate only (it can
-        see display.hardware; a plugin cannot). The frame hold is NOT applied
-        here -- see the note in the body. The caller must pass
-        ``settings.frame_hold`` to ``display_manager.set_scrolling_state(True,
-        ...)`` when it starts scrolling, or a sub-refresh speed still presents
-        a new frame every refresh and the motion falls back to fractional
-        pixels.
-    :param snap_to_crisp: move the requested speed to the nearest speed the
-        panel can show in whole pixels. On by default because a speed that does
-        not divide evenly has no good rendering, only a choice of artefacts.
-
-    :returns: the settings applied, so the caller can log or assert on them.
-    """
-    log = plugin_logger or logger
-
-    # Refresh rate, most authoritative first: what the caller passed, then the
-    # display manager (which can see display.hardware; a plugin cannot), then
-    # the global config, then the default.
-    #
-    # This has to be settled BEFORE resolve(), not after. resolve() uses the
-    # refresh to fill in target_fps, pixels_per_frame and the judder warning,
-    # so deriving it afterwards described a 100Hz panel to everyone running at
-    # 60 -- and with snap_to_crisp=False nothing downstream corrected it, so
-    # set_target_fps() paced the helper to 100 FPS on a 60Hz panel.
-    hz = _coerce(refresh_hz)
-    if hz is None and display_manager is not None:
-        hz = _coerce(getattr(display_manager, "refresh_hz", None))
-    if hz is None:
-        hz = refresh_hz_from_config(global_config)
-
-    settings = resolve(
-        plugin_config,
-        global_config,
-        default_pixels_per_second=default_pixels_per_second,
-        refresh_hz=hz,
-    )
-    applied = settings.pixels_per_second
-    choice = None
-
-    if snap_to_crisp:
-        choice = solve_crisp(settings.pixels_per_second, hz)
-        applied = choice.pixels_per_second
-        settings = replace(
-            settings,
-            pixels_per_second=applied,
-            requested_pixels_per_second=settings.pixels_per_second,
-            crisp=choice,
-            pixels_per_frame=float(choice.pixels_per_frame),
-            # Snapping resolves the whole-pixel problem the warning describes.
-            warning=None if settings.warning and "judder" in settings.warning
-            else settings.warning,
-        )
-
-    if hasattr(scroll_helper, "set_frame_based_scrolling"):
-        scroll_helper.set_frame_based_scrolling(False)
-    scroll_helper.set_scroll_speed(applied)
-    if choice and hasattr(scroll_helper, "set_target_fps"):
-        scroll_helper.set_target_fps(choice.frames_per_second)
-    elif settings.target_fps and hasattr(scroll_helper, "set_target_fps"):
-        scroll_helper.set_target_fps(settings.target_fps)
-
-    # Deliberately NOT applied here. The hold belongs to a scroll, not to a
-    # plugin's lifetime: plugins share one display manager, and one left set at
-    # construction is reset the moment any other plugin finishes scrolling.
-    # Callers pass settings.frame_hold to set_scrolling_state(True, ...) when
-    # they start scrolling. configure() only reports what is needed.
-
-    if choice:
-        requested = settings.requested_pixels_per_second
-        if abs(requested - applied) > 0.05:
-            log.info(
-                "Scroll configured: %s (asked for %.1f px/s from %s; "
-                "nearest whole-pixel speed on a %.0fHz panel)",
-                choice.describe(), requested, settings.source, hz,
-            )
-        else:
-            log.info("Scroll configured: %s (from %s)",
-                     choice.describe(), settings.source)
-        if choice.frame_hold > 1:
-            log.debug(
-                "Scroll needs a frame hold of %d - pass settings.frame_hold to "
-                "display_manager.set_scrolling_state(True, ...) each scroll",
-                choice.frame_hold,
-            )
-    else:
-        log.info("Scroll configured: %s", settings.describe())
-
-    if settings.warning:
-        log.warning("Scroll speed: %s", settings.warning)
-    return settings
-
-
-def refresh_hz_from_config(global_config: Optional[Dict[str, Any]]) -> float:
-    """The panel's refresh cap from the global config, or the default."""
-    if not isinstance(global_config, dict):
-        return DEFAULT_REFRESH_HZ
-    # Each level is checked for being a mapping rather than merely truthy: a
-    # malformed config where display or display.hardware is a string or a list
-    # raised AttributeError out of what is meant to be a total function with a
-    # default, taking down every caller that asked for the refresh rate.
-    display = global_config.get("display")
-    if not isinstance(display, dict):
-        return DEFAULT_REFRESH_HZ
-    hardware = display.get("hardware")
-    if not isinstance(hardware, dict):
-        return DEFAULT_REFRESH_HZ
-    return _coerce(hardware.get("limit_refresh_rate_hz")) or DEFAULT_REFRESH_HZ

src/common/scroll_helper.py

@@ -16,7 +16,6 @@ Features:
 """
 
 import logging
-import math
 import time
 from typing import Optional, Dict, Any
 from PIL import Image
@@ -30,49 +29,6 @@ except ImportError:
     HAS_SCIPY = False
 
 
-def frame_stats(frame_times: list) -> Dict[str, Any]:
-    """Summary statistics over one window of frame durations (seconds).
-
-    Split out of log_frame_rate() so the arithmetic can be tested without a
-    clock. Median and p95 are the real ones: the median takes both middle
-    samples on an even window, and p95 is nearest-rank, so a 100-frame window
-    reports the 95th sorted sample rather than the 96th. That matters twice
-    over, because the median is also the threshold the stall and skip counts
-    are measured against.
-    """
-    window = sorted(frame_times)
-    n = len(window)
-    median = (window[n // 2] if n % 2
-              else (window[n // 2 - 1] + window[n // 2]) / 2.0)
-    mean = sum(window) / n
-    # Anything past 1.5x the median missed a panel refresh; anything under
-    # half of it never reached the panel at all (dirty tracking skipped the
-    # swap, so the frame did not wait for vsync).
-    return {
-        "frames": n,
-        "fps": (1.0 / mean) if mean > 0 else 0.0,
-        "median": median,
-        "p95": window[max(0, math.ceil(0.95 * n) - 1)],
-        "max": window[-1],
-        "min": window[0],
-        "stalls": sum(1 for f in window if f > median * 1.5),
-        "skips": sum(1 for f in window if f < median * 0.5),
-    }
-
-
-def format_frame_stats(frame_times: list) -> str:
-    """The one-line rendering of frame_stats(), in milliseconds."""
-    s = frame_stats(frame_times)
-    n = s["frames"]
-    return (
-        f"{s['fps']:.1f} fps over {n} frames | "
-        f"median {s['median'] * 1000:.2f}ms p95 {s['p95'] * 1000:.2f}ms "
-        f"max {s['max'] * 1000:.2f}ms min {s['min'] * 1000:.2f}ms | "
-        f"stalls {s['stalls']} ({100.0 * s['stalls'] / n:.1f}%) "
-        f"skips {s['skips']} ({100.0 * s['skips'] / n:.1f}%)"
-    )
-
-
 class ScrollHelper:
     """
     Helper class for scrolling text and image content on LED displays.
@@ -119,19 +75,8 @@ class ScrollHelper:
         # Pre-allocated buffer for output frame (reused to avoid allocations)
         self._frame_buffer: Optional[np.ndarray] = None
         
-        # Sub-pixel scrolling: OFF by default, and that is deliberate.
-        # Blending renders a half-step by mixing two adjacent columns 50/50.
-        # On a high-resolution screen that reads as smooth motion; on a coarse
-        # LED matrix showing pixel-font text it does not. A one-pixel stroke
-        # becomes two half-brightness pixels, so frames alternate between crisp
-        # and smeared and the text appears to shimmer and jump a pixel ahead --
-        # tested on a 2x128x64 panel and clearly worse than integer stepping.
-        #
-        # The rule this display obeys: motion is smooth when it advances a
-        # whole number of pixels per refresh. Anything slower must either
-        # blend (blur) or repeat frames (judder); blending is the worse of the
-        # two here. Vegas mode still opts in via set_sub_pixel_scrolling().
-        self.sub_pixel_scrolling = False
+        # Sub-pixel scrolling settings (disabled - using high FPS integer scrolling instead)
+        self.sub_pixel_scrolling = False  # Disabled - use high frame rate for smoothness
         self._last_integer_position = 0  # Cache for integer position to avoid repeated calculations
         
         # Frame-based scrolling settings
@@ -160,9 +105,6 @@ class ScrollHelper:
         self.last_frame_time = time.time()
         self.last_fps_log_time = time.time()
         self.frame_times = []
-        # Every frame time since the last stats line, so the 5s summary can
-        # report the tail rather than one arbitrary sample. Cleared on log.
-        self._window: list = []
         
         # Scrolling state management
         self.is_scrolling = False
@@ -302,31 +244,19 @@ class ScrollHelper:
             if self.last_step_time == 0.0:
                 self.last_step_time = current_time
             
-            # Frame-based mode advances by elapsed time, exactly like the
-            # time-based branch below, at the same configured speed
-            # (scroll_speed px per scroll_delay seconds).
-            #
-            # It used to step discretely: 0, 1 or 2 whole pixels depending on
-            # whether a wall clock had passed scroll_delay. Plugins set
-            # scroll_delay to the target frame period, so that comparison sits
-            # exactly on its own threshold and the decision flips on sub-
-            # millisecond jitter -- a frame a hair early moved nothing and
-            # rendered an identical frame, a frame a hair late moved two
-            # pixels. Rounding the step count fixed the stalls but still
-            # discarded the remainder, so the error never corrected.
-            #
-            # Accumulating elapsed time keeps position exactly proportional to
-            # real time: jitter shifts a pixel boundary by a fraction of a
-            # frame instead of flipping a whole step, and nothing is lost or
-            # gained. This is what the one visibly smooth scroller on the
-            # hardware (the stock ticker) was already doing by virtue of never
-            # enabling frame-based mode.
-            if self.scroll_delay > 0:
-                pixels_per_second = self.scroll_speed / self.scroll_delay
+            # Check if scroll_delay has passed
+            time_since_last_step = current_time - self.last_step_time
+            if time_since_last_step >= self.scroll_delay:
+                # Move pixels (can move multiple steps if lag occurred, but cap to prevent huge jumps)
+                steps = int(time_since_last_step / self.scroll_delay)
+                # Cap at reasonable number to prevent huge jumps from lag
+                max_steps = max(1, int(0.04 / self.scroll_delay))  # Limit to 0.04s (2 steps at 50 FPS) for smoother scrolling
+                steps = min(steps, max_steps)
+                pixels_to_move = self.scroll_speed * steps
+                # Update last_step_time, preserving fractional delay for smooth timing
+                self.last_step_time = current_time - (time_since_last_step % self.scroll_delay)
             else:
-                pixels_per_second = self.scroll_speed * 100.0
-            pixels_to_move = pixels_per_second * delta_time
-            self.last_step_time = current_time
+                pixels_to_move = 0.0
         else:
             # Time-based: move based on time delta (correct speed over time)
             # scroll_speed is pixels per second
@@ -1088,24 +1018,17 @@ class ScrollHelper:
         if len(self.frame_times) > 100:
             self.frame_times.pop(0)
         
-        # Every frame since the last log, not just the last 100 and not just
-        # the one that happens to land on the 5s boundary. The old line
-        # reported a single instantaneous sample -- roughly 1 frame in 500 --
-        # which cannot see a stall that hits 1% of frames, and reported it
-        # next to an average that hides the same stall by construction (a 2ms
-        # duplicate and a 21ms double-wait mean exactly 10ms). Chasing scroll
-        # judder needs the tail, so keep the window and report percentiles.
-        self._window.append(frame_time)
-        
         # Log FPS every 5 seconds to avoid spam
         if current_time - self.last_fps_log_time >= 5.0:
-            self.logger.info(
-                "Scroll frame stats - %s",
-                format_frame_stats(self._window or [frame_time]),
-            )
+            avg_frame_time = sum(self.frame_times) / len(self.frame_times)
+            avg_fps = 1.0 / avg_frame_time if avg_frame_time > 0 else 0
+            instant_fps = 1.0 / frame_time if frame_time > 0 else 0
+            
+            self.logger.info(f"Scroll frame stats - Avg FPS: {avg_fps:.1f}, "
+                           f"Current FPS: {instant_fps:.1f}, "
+                           f"Frame time: {frame_time*1000:.2f}ms")
             self.last_fps_log_time = current_time
             self.frame_count = 0
-            self._window = []
         
         self.last_frame_time = current_time
         self.frame_count += 1

src/display_controller.py

@@ -198,10 +198,6 @@ class DisplayController:
         # the main run loop reconciles (loads/unloads) on its own thread so
         # mutating available_modes never races with rendering.
         self._pending_plugin_reconcile = False
-        # Monotonic stamp of the last mailbox disk read; see
-        # _poll_on_demand_requests. None means "never polled", so the first
-        # call always goes through.
-        self._last_on_demand_poll: Optional[float] = None
         self.on_demand_active = False
         self.on_demand_mode: Optional[str] = None
         self.on_demand_modes: List[str] = []  # All modes for the on-demand plugin
@@ -1271,34 +1267,12 @@ class DisplayController:
         self.on_demand_schedule_override = False
         self._publish_on_demand_state()
 
-    #: Shortest gap between mailbox disk reads. This is called after every
-    #: frame -- about 125 times a second on a scrolling mode -- and the read
-    #: below is deliberately uncached, so without a floor it was 125 disk reads
-    #: per second to find nothing. An on-demand request comes from a person
-    #: clicking in the web UI, so a quarter second of latency is not
-    #: perceptible, and it cuts the read rate by 30x.
-    ON_DEMAND_POLL_INTERVAL = 0.25
-
     def _poll_on_demand_requests(self) -> None:
         """Poll cache for new on-demand requests from external controllers."""
-        now = time.monotonic()
-        if (self._last_on_demand_poll is not None
-                and now - self._last_on_demand_poll < self.ON_DEMAND_POLL_INTERVAL):
-            return
-        self._last_on_demand_poll = now
-
         try:
             # Use a long max_age (1 hour) to ensure requests aren't expired before processing
-            # The request_id check prevents duplicate processing.
-            #
-            # memory_ttl=0 is required, not optional: this key is a mailbox the
-            # web process writes and this process reads. get() defaults the
-            # in-memory TTL to max_age, so without it the first request read was
-            # pinned in memory for the full hour and every later poll returned
-            # that stale copy -- meaning no second on-demand request was honoured
-            # for an hour, while the API still reported success.
-            request = self.cache_manager.get('display_on_demand_request',
-                                             max_age=3600, memory_ttl=0)
+            # The request_id check prevents duplicate processing
+            request = self.cache_manager.get('display_on_demand_request', max_age=3600)
         except (OSError, RuntimeError, ValueError, TypeError) as err:
             logger.error("Failed to read on-demand request: %s", err, exc_info=True)
             return
@@ -1344,36 +1318,6 @@ class DisplayController:
         # Mark as processed BEFORE processing (to prevent duplicate processing)
         self.cache_manager.set('display_on_demand_processed_id', request_id, ttl=3600)
         self.on_demand_request_id = request_id
-        # Consume the mailbox entry. Leaving it on disk meant a restart replayed
-        # the previous request: the fresh controller read it, activated it and
-        # cached it, so the request the caller had just made was ignored and the
-        # panel silently showed the earlier plugin. processed_id still guards
-        # against double-processing if this delete fails.
-        try:
-            # Compare before deleting. The web process can post a newer request
-            # between the read above and this delete; an unconditional delete
-            # threw that one away and it was never processed -- the user's
-            # second click did nothing. Re-reading uncached and only deleting
-            # our own request_id means a newer request is left in the mailbox
-            # for the next poll instead.
-            #
-            # This narrows the window rather than closing it: a request landing
-            # between this re-read and the delete is still lost. Closing it
-            # properly needs an atomic claim (a rename, or a compare-and-delete
-            # primitive) that the cache layer does not currently offer, so the
-            # honest fix is a smaller window plus this note, not a bigger lock.
-            current = self.cache_manager.get('display_on_demand_request',
-                                             max_age=3600, memory_ttl=0)
-            if not current or current.get('request_id') == request_id:
-                self.cache_manager.delete('display_on_demand_request')
-            else:
-                logger.debug("Newer on-demand request %s arrived while processing "
-                             "%s; leaving it in the mailbox",
-                             current.get('request_id'), request_id)
-        except (OSError, AttributeError, KeyError) as err:
-            # Best-effort: processed_id still guards against reprocessing if the
-            # mailbox cannot be cleared.
-            logger.debug("Could not clear the on-demand request mailbox: %s", err)
         
         if action == 'start':
             logger.info("Processing on-demand start request for plugin: %s", request.get('plugin_id'))
@@ -2461,7 +2405,6 @@ class DisplayController:
                             )
 
                             while True:
-                                _frame_start = time.perf_counter()
                                 try:
                                     with self._display_lock_or_skip(plugin_id) as can_display:
                                         if can_display:
@@ -2482,26 +2425,11 @@ class DisplayController:
                                 # Multi-display sync: send follower frame after each render
                                 self._send_follower_frame(manager_to_display)
 
+                                time.sleep(display_interval)
                                 self._tick_plugin_updates()
                                 self._poll_on_demand_requests()
                                 self._check_on_demand_expiration()
 
-                                # Pace to the frame deadline rather than sleeping a flat
-                                # interval on top of the work. display() has already
-                                # blocked on the panel's vsync by this point, so an
-                                # unconditional sleep is added to a wait that already
-                                # happened. Measured on a 2x128x64 chain at
-                                # limit_refresh_rate_hz=100: ~4ms of render plus a flat
-                                # 8ms put each iteration at ~12ms against a 10ms refresh
-                                # grid, so every swap missed a refresh and the loop
-                                # settled at 50fps where display_interval asks for 125 --
-                                # and with zero headroom, ~14% of frames slipped a
-                                # further refresh, which is what reads as scroll stutter.
-                                _remaining = display_interval - (time.perf_counter() - _frame_start)
-                                # Yield even when the frame overran its budget, so plugin
-                                # update threads and the web UI are not starved of the GIL.
-                                time.sleep(_remaining if _remaining > 0 else 0.001)
-
                                 if self.current_display_mode != active_mode:
                                     logger.debug("Mode changed during high-FPS loop, breaking early")
                                     break

src/display_manager.py

@@ -240,14 +240,6 @@ class DisplayManager:
         self._update_lock = threading.RLock()
         
         # Scrolling state tracking for graceful updates
-        # How many panel refreshes each pushed frame is held for. 1 means a new
-        # frame every refresh. Higher values are how a scroll runs slower than
-        # one pixel per refresh WITHOUT fractional pixel positions: the panel
-        # keeps refreshing at full rate (so flicker is unchanged) but motion
-        # advances a whole pixel every Nth refresh instead of every one.
-        # See src/common/scroll_config.py and scripts/scroll_speeds.py.
-        self._frame_hold = 1
-
         self._scrolling_state = {
             'is_scrolling': False,
             'last_scroll_activity': 0,
@@ -779,33 +771,16 @@ class DisplayManager:
                     return  # Skip hardware write — content is being captured off-screen
 
                 digest = None
-                frame_checksum = None
                 if self._dirty_tracking_enabled:
                     try:
                         brightness = getattr(self.matrix, 'brightness', None)
                     except AttributeError:
                         brightness = None
-                    frame_checksum = zlib.adler32(self.image.tobytes())
-                    digest = (frame_checksum, brightness)
-                    if digest == self._last_pushed_digest and not self.is_currently_scrolling():
+                    digest = (zlib.adler32(self.image.tobytes()), brightness)
+                    if digest == self._last_pushed_digest:
                         # Nothing changed since the last push — the panel is
                         # already showing exactly this frame.
-                        #
-                        # Never taken mid-scroll, and that exception is the
-                        # point. SwapOnVSync is what paces the render loop, so
-                        # skipping it also skips the wait: a duplicate frame
-                        # returns in ~8ms instead of ~10ms on a 100Hz panel,
-                        # advances only 0.8px instead of 1.0px, and so makes
-                        # the *next* frame more likely to repeat as well. That
-                        # is self-sustaining -- measured at ~20% duplicate
-                        # frames mid-scroll on the odds ticker, against
-                        # essentially zero on a lighter plugin with identical
-                        # scroll settings. Swapping an identical frame costs
-                        # one canvas copy and keeps the loop locked to the
-                        # panel; falling out of that lock costs smooth motion.
-                        # Static content is unaffected: is_currently_scrolling()
-                        # expires on its own inactivity threshold.
-                        self._write_snapshot_if_due(frame_checksum)
+                        self._write_snapshot_if_due()
                         return
 
                 # Copy the current image to the offscreen canvas. In double-sided
@@ -815,10 +790,8 @@ class DisplayManager:
                 else:
                     self.offscreen_canvas.SetImage(self.image)
 
-                # Swap buffers immediately. framerate_fraction holds the frame
-                # for N refreshes; SwapOnVSync blocks for all of them, which is
-                # what paces the render loop to the chosen frame rate.
-                self.matrix.SwapOnVSync(self.offscreen_canvas, self._frame_hold)
+                # Swap buffers immediately
+                self.matrix.SwapOnVSync(self.offscreen_canvas)
 
                 # Swap our canvas references
                 self.offscreen_canvas, self.current_canvas = self.current_canvas, self.offscreen_canvas
@@ -826,7 +799,7 @@ class DisplayManager:
                 self._last_pushed_digest = digest
 
                 # Write a snapshot for the web preview (throttled)
-                self._write_snapshot_if_due(frame_checksum)
+                self._write_snapshot_if_due()
         except Exception as e:
             logger.error(f"Error updating display: {e}")
 
@@ -1307,65 +1280,12 @@ class DisplayManager:
         
         return dt.strftime(f"%b %-d{suffix}") 
 
-    @property
-    def refresh_hz(self) -> float:
-        """The panel's refresh rate in Hz, from the hardware config.
-
-        The authoritative place to ask, because a plugin only receives its own
-        config section and cannot see display.hardware. Scroll pacing needs
-        this: the speeds a panel can show in whole pixels are refresh_hz
-        divided by the frame hold, so getting it wrong silently produces
-        fractional-pixel motion. See src/common/scroll_config.py.
-
-        Note this is the configured *cap*, not necessarily what the panel
-        achieves -- scripts/scroll_speeds.py --measure reports the real rate.
-        """
-        hardware = (self.config.get('display') or {}).get('hardware') or {}
-        try:
-            value = float(hardware.get('limit_refresh_rate_hz') or 0)
-        except (TypeError, ValueError):
-            value = 0.0
-        return value if value > 0 else 100.0
-
-    def set_frame_hold(self, refreshes: int) -> None:
-        """Hold each pushed frame for this many panel refreshes (>=1).
-
-        Set by the scroll configuration so a plugin can run at, say, 50px/s on
-        a 100Hz panel as one whole pixel every second refresh, rather than half
-        a pixel every refresh (which has to be blended or repeated unevenly).
-
-        Reset to 1 whenever scrolling stops, so one plugin's pacing cannot
-        leak into the next thing on screen.
-        """
-        try:
-            value = int(refreshes)
-        except (TypeError, ValueError):
-            logger.warning("Ignoring unusable frame hold: %r", refreshes)
-            return
-        self._frame_hold = max(1, min(255, value))
-
-    def set_scrolling_state(self, is_scrolling: bool, frame_hold: int = 1):
-        """Set the current scrolling state, and this scroll's frame pacing.
-
-        Call this when a display starts or stops scrolling. ``frame_hold`` is
-        how many panel refreshes each frame is held for -- 2 gives one whole
-        pixel every second refresh, which is how a scroll runs at half the
-        refresh rate without fractional pixel positions.
-
-        The hold is set here rather than once at plugin construction because
-        it must not outlive the scroll that asked for it: plugins share one
-        display manager, so a hold left set by whoever scrolled last would
-        silently re-pace the next plugin. Passing it alongside the state makes
-        the lifetime exactly the scroll, and the default of 1 means any caller
-        that does not care gets a new frame every refresh.
-        """
+    def set_scrolling_state(self, is_scrolling: bool):
+        """Set the current scrolling state. Call this when a display starts/stops scrolling."""
         current_time = time.time()
         self._scrolling_state['is_scrolling'] = is_scrolling
         if is_scrolling:
             self._scrolling_state['last_scroll_activity'] = current_time
-            self.set_frame_hold(frame_hold)
-        else:
-            self._frame_hold = 1
         logger.debug(f"Scrolling state set to: {is_scrolling}")
 
     def is_currently_scrolling(self) -> bool:
@@ -1496,20 +1416,11 @@ class DisplayManager:
                 self._viewer_fresh = False
         return self._viewer_fresh
 
-    def _write_snapshot_if_due(self, frame_checksum: Optional[int] = None) -> None:
+    def _write_snapshot_if_due(self) -> None:
         """Mirror the current frame to the preview snapshot when the policy
         says it's worth it — see src/common/snapshot_policy.py. Unchanged
         frames are never re-encoded; without viewers the cadence drops to
-        the idle keepalive.
-
-        Args:
-            frame_checksum: adler32 of the current frame, when the caller has
-                already computed one. Dirty tracking checksums every frame a
-                few lines above the call site, and re-deriving it here meant a
-                second tobytes() plus a second pass over the whole framebuffer
-                on every single frame — ~0.17ms per frame of the two combined
-                at 256x64, paid 100 times a second to reach the same number.
-        """
+        the idle keepalive."""
         try:
             now = time.time()
             viewer_fresh = self._viewer_is_fresh(now)
@@ -1519,8 +1430,7 @@ class DisplayManager:
                 self._last_snapshot_ts = 0.0
             self._viewer_was_fresh = viewer_fresh
 
-            digest = (frame_checksum if frame_checksum is not None
-                      else zlib.adler32(self.image.tobytes()))
+            digest = zlib.adler32(self.image.tobytes())
             action = snapshot_policy.decide(
                 now, self._last_snapshot_ts, self._last_snapshot_touch_ts,
                 viewer_fresh, digest != self._last_snapshot_digest)
@@ -1543,30 +1453,12 @@ class DisplayManager:
                 if parent_dir and str(parent_dir) != '/tmp':  # nosec B108 - guard to skip /tmp for permission ops
                     ensure_directory_permissions(parent_dir, get_assets_dir_mode())
                 self._snapshot_dir_prepared = True
-            # Write atomically: temp then replace. The temp name must be
-            # unique, not "<snapshot>.tmp": /tmp is world-writable and sticky,
-            # and this file is written by whichever user the display service
-            # runs as while tests and tooling run as someone else. A leftover
-            # fixed-name temp owned by another user is then unopenable even by
-            # root (fs.protected_regular refuses O_CREAT on a foreign file in a
-            # sticky dir), which froze the preview and the health check's
-            # liveness proxy until somebody deleted it by hand. Same pattern as
-            # the hardware-status write above.
-            _fd, tmp_path = tempfile.mkstemp(
-                dir=str(snapshot_path_obj.parent),
-                prefix=f".{snapshot_path_obj.name}.", suffix=".tmp")
+            # Write atomically: temp then replace
+            tmp_path = f"{self._snapshot_path}.tmp"
+            self.image.save(tmp_path, format='PNG')
             try:
-                with os.fdopen(_fd, "wb") as _f:
-                    self.image.save(_f, format='PNG')
-                os.chmod(tmp_path, 0o644)
                 os.replace(tmp_path, self._snapshot_path)
             except Exception:
-                # Never leave the temp behind -- that is what made the failure
-                # permanent rather than transient.
-                try:
-                    os.unlink(tmp_path)
-                except OSError:
-                    pass
                 # Fallback to direct save if replace not supported
                 self.image.save(self._snapshot_path, format='PNG')
             # Set proper file permissions after saving

src/font_manager.py

@@ -96,8 +96,7 @@ class FontManager:
         self.common_fonts = {
             "press_start": "assets/fonts/PressStart2P-Regular.ttf",
             "four_by_six": "assets/fonts/4x6-font.ttf",
-            "five_by_seven": "assets/fonts/5x7.bdf",
-            "tom_thumb": "assets/fonts/tom-thumb.bdf"
+            "five_by_seven": "assets/fonts/5x7.bdf"
             # Note: cozette_bdf removed - font file not available
             # To re-enable: download cozette.bdf from https://github.com/the-moonwitch/Cozette
             # and add: "cozette_bdf": "assets/fonts/cozette.bdf"

src/plugin_system/store_manager.py

@@ -1589,7 +1589,6 @@ class PluginStoreManager:
             with open(manifest_path, 'r') as f:
                 manifest = json.load(f)
             
-            requested_id = plugin_id
             plugin_id = plugin_id or manifest.get('id')
             if not plugin_id:
                 return {
@@ -1665,15 +1664,6 @@ class PluginStoreManager:
             
             branch_info = f" (branch: {branch_used})" if branch_used else ""
             self.logger.info(f"Successfully installed plugin from URL: {plugin_id}{branch_info}")
-            # User deliberately (re)installed this plugin -- clear any persistent
-            # uninstall record, exactly as install_plugin() does. Without this the
-            # id stays in config/uninstalled_plugins.json and
-            # purge_uninstalled_plugins(), which runs at every web-app startup,
-            # deletes the directory again: the plugin works for the rest of the
-            # session and is gone after the next reboot.
-            self.forget_uninstalled_plugin(
-                *(pid for pid in (requested_id, plugin_id, manifest.get('id')) if pid)
-            )
             result = {
                 'success': True,
                 'plugin_id': plugin_id,
@@ -2434,65 +2424,6 @@ class PluginStoreManager:
         except (OSError, ValueError):
             pass
         
-        # Last resort: the directory name may differ from the id being looked
-        # up. install_plugin() deliberately renames a plugin's directory to the
-        # MANIFEST id when it differs from the REGISTRY id (see the rename near
-        # "doesn't match registry ID" above), so `stocks` in the registry lands
-        # in `ledmatrix-stocks/`. Every lookup above is by directory name, so
-        # update_plugin("stocks") found nothing and reported the plugin as not
-        # installed -- silently, and for good: the user sees no error and stays
-        # on a stale version. Four installed plugins hit this in practice
-        # (leaderboard, music, stocks, weather).
-        #
-        # Deliberately last so the two lookups above keep their exact meaning;
-        # this only runs when a direct hit already failed. See
-        # test_discovery_path_contract.py, which pins that ordering.
-        for search_dir in self._candidate_plugin_dirs():
-            match = self._find_by_manifest_id(search_dir, plugin_id)
-            if match is not None:
-                self.logger.debug(
-                    "Resolved plugin '%s' to %s via its manifest id "
-                    "(directory name differs from the id)", plugin_id, match)
-                return match
-
-        return None
-
-    def _candidate_plugin_dirs(self) -> List[Path]:
-        """Directories that may hold installed plugins, configured one first."""
-        dirs = [self.plugins_dir]
-        try:
-            base = self.plugins_dir if self.plugins_dir.is_absolute()                 else self.plugins_dir.resolve()
-            sibling = base.parent / 'plugins'
-            if sibling != self.plugins_dir:
-                dirs.append(sibling)
-        except (OSError, ValueError):
-            pass
-        return [d for d in dirs if d.exists()]
-
-    @staticmethod
-    def _find_by_manifest_id(search_dir: Path, plugin_id: str) -> Optional[Path]:
-        """A subdirectory of `search_dir` whose manifest declares `plugin_id`.
-
-        Skips half-finished installs: store_manager renames a directory aside
-        with '.standalone-backup-' during install and rollback, and treating
-        one as installed would resurrect a ghost plugin.
-        """
-        try:
-            entries = sorted(search_dir.iterdir())
-        except (OSError, ValueError):
-            return None
-        for entry in entries:
-            if not entry.is_dir() or '.standalone-backup-' in entry.name:
-                continue
-            manifest = entry / 'manifest.json'
-            if not manifest.is_file():
-                continue
-            try:
-                with open(manifest, 'r', encoding='utf-8') as handle:
-                    if json.load(handle).get('id') == plugin_id:
-                        return entry
-            except (OSError, ValueError):
-                continue
         return None
     
     _SKIN_ID_PATTERN = re.compile(r'^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$')

src/plugin_system/testing/harness.py

@@ -14,8 +14,6 @@ via BoundsCheckingDisplayManager, and golden-image comparison.
 import contextlib
 import http.client
 import inspect
-import time
-from datetime import timedelta
 import socket
 import ssl
 import urllib.error
@@ -27,7 +25,7 @@ from PIL import Image, ImageChops
 
 from src.logging_config import get_logger
 from .bounds_display_manager import BoundsCheckingDisplayManager
-from .loading import load_config_defaults, load_manifest, merge_config
+from .loading import load_config_defaults, load_manifest
 from .sizes import DEFAULT_TEST_SIZES, safe_mode_filename, size_label
 
 logger = get_logger("[Plugin Harness]")
@@ -118,20 +116,11 @@ def list_modes(plugin_instance: Any, manifest: Dict[str, Any], plugin_id: str) -
 
 def _instantiate(plugin_id: str, manifest: Dict[str, Any], plugin_dir: Path,
                  config: Dict[str, Any], mock_data: Dict[str, Any],
-                 display_manager: Any, cache_manager: Any = None) -> Any:
-    """Load and construct a plugin instance with mocked managers.
-
-    Pass ``cache_manager`` to share one cache across the renders of a plugin.
-    Building a fresh one per (size, mode) made every render a cold start, so a
-    plugin that fetches per game or per player re-fetched everything N times --
-    baseball-scoreboard took 840s for nine renders where ~72s was the arithmetic
-    -- and the cache-hit path, which is what a running rig executes almost
-    always, was never exercised.
-    """
+                 display_manager: Any) -> Any:
+    """Load and construct a plugin instance with mocked managers."""
     from src.plugin_system.plugin_loader import PluginLoader
     from src.plugin_system.testing import MockCacheManager, MockPluginManager
 
-    if cache_manager is None:
     cache_manager = MockCacheManager()
     for key, value in (mock_data or {}).items():
         cache_manager.set(key, value)
@@ -171,107 +160,6 @@ def _render_mode(plugin_instance: Any, mode: str) -> Any:
     return plugin_instance.display(force_clear=False)
 
 
-# How many extra frames to drive before believing a mode really draws nothing.
-# A scroll starts with its content off-panel, so frame 1 is legitimately blank;
-# measured across the fleet, content appears by frame 2-4 (f1-scoreboard),
-# frame 4 (ledmatrix-elections) and frame 38 at 64px (ledmatrix-leaderboard).
-EMPTY_RECHECK_FRAMES = 48
-# Seconds to advance the clock between those frames. Scroll position is usually
-# driven by elapsed time, which a frozen clock never provides.
-EMPTY_RECHECK_STEP = 0.05
-
-
-def _has_content(image) -> bool:
-    """True when any pixel is lit above the threshold."""
-    if image is None:
-        return False
-    return image.convert("L").point(
-        lambda p: 255 if p > _LIT_THRESHOLD else 0).getbbox() is not None
-
-
-def _render_mode_again(plugin_instance: Any, mode: str) -> Any:
-    """Draw one more frame WITHOUT force_clear.
-
-    _render_mode passes force_clear=True, which for a scrolling plugin means
-    "reset the scroll to the start" -- so repeating it would redraw frame 1 for
-    ever. The re-check needs the plugin to advance.
-    """
-    sig = inspect.signature(plugin_instance.display)
-    if "display_mode" in sig.parameters:
-        return plugin_instance.display(force_clear=False, display_mode=mode)
-    return plugin_instance.display(force_clear=False)
-
-
-def _settle_empty_frame(inst, mode, dm, result, freezer) -> None:
-    """Give an apparently-empty mode a few frames to draw before believing it.
-
-    One frame is not evidence: a scroll's first frame is its blank scroll-in
-    buffer. Without this, every scrolling plugin was warned about -- 60 of 76
-    warnings on a 44-plugin rig were false, which is the rate at which people
-    stop reading a warning.
-    """
-    if result.error is not None or result.display_returned is False:
-        return
-    if _has_content(result.image):
-        return
-    # The frozen clock is shared by every render in the matrix, so any time this
-    # probe borrows has to be given back -- otherwise a mode that scrolls in
-    # leaves the clock advanced and every later mode renders at the wrong
-    # instant, drifting its golden. Seen as 5 spurious f1_upcoming drifts.
-    resume_at = None
-    if freezer is not None:
-        try:
-            resume_at = freezer()
-        except (AttributeError, TypeError, ValueError):
-            # Not a freezegun factory, or a version whose factory is not
-            # callable. Only used to restore the clock, never load-bearing.
-            resume_at = None
-    try:
-        _settle_loop(inst, mode, dm, result, freezer)
-    finally:
-        if resume_at is not None:
-            try:
-                freezer.move_to(resume_at)
-            except (AttributeError, TypeError, ValueError):
-                pass
-
-
-def _settle_loop(inst, mode, dm, result, freezer) -> None:
-    tick = getattr(freezer, "tick", None) if freezer is not None else None
-    for _ in range(EMPTY_RECHECK_FRAMES):
-        if tick is not None:
-            # timedelta rather than a bare float: freezegun has accepted a
-            # number only since 1.x, and a stale pin would raise here.
-            try:
-                tick(timedelta(seconds=EMPTY_RECHECK_STEP))
-            except (AttributeError, TypeError, ValueError):
-                # Pacing is best-effort; a freezegun that will not take a
-                # timedelta just means this probe runs without advancing time.
-                pass
-        else:
-            # No frozen clock, so the real one has to do the advancing. Without
-            # this the 48 frames run in microseconds, elapsed time stays ~0, and
-            # a scroll driven by elapsed time never moves -- which is exactly
-            # the plugin this check is trying not to slander.
-            time.sleep(EMPTY_RECHECK_STEP)
-        try:
-            result.display_returned = _render_mode_again(inst, mode)
-        except Exception as e:  # noqa: BLE001
-            # Deliberately broad: this calls a plugin's display(), which can
-            # raise anything. Recorded rather than swallowed -- a mode that
-            # renders one good frame and then crashes on the next is broken,
-            # and returning silently here reported it as passing. The frame
-            # already captured stays on the result so the failure is still
-            # inspectable.
-            result.error = repr(e)
-            return
-        image = dm.get_image()
-        if _has_content(image):
-            result.image = image
-            result.overflow = dm.check_overflow()
-            return
-
-
 def _freeze(freeze_time: Optional[str]):
     """Context manager that freezes wall-clock time when freeze_time is given,
     so time-dependent plugins (clocks, countdowns) render deterministic goldens."""
@@ -305,9 +193,7 @@ def render_plugin_matrix(
     manifest = load_manifest(plugin_dir)
     # Start from config_schema.json defaults so the plugin behaves like a real
     # install; explicit caller config still wins over a schema default.
-    config = merge_config(
-        merge_config({"enabled": True}, load_config_defaults(plugin_dir)),
-        config or {})
+    config = {"enabled": True, **load_config_defaults(plugin_dir), **(config or {})}
     sizes = sizes or DEFAULT_TEST_SIZES
     results: List[RenderResult] = []
 
@@ -316,35 +202,25 @@ def render_plugin_matrix(
     # rendering a smaller one, instead of being clipped into a false pass.
     extent = (max(w for w, _ in sizes), max(h for _, h in sizes))
 
-    # One cache for the whole matrix: see _instantiate. The display manager
-    # stays per-render (the bounds checking depends on that); only fetched data
-    # is shared.
-    from src.plugin_system.testing import MockCacheManager
-    cache_manager = MockCacheManager()
-    for key, value in (mock_data or {}).items():
-        cache_manager.set(key, value)
-
-    with _freeze(freeze_time) as freezer:
+    with _freeze(freeze_time):
         for width, height in sizes:
             results.extend(_render_size(
                 plugin_id, manifest, plugin_dir, config, mock_data or {},
-                width, height, run_update, extent, cache_manager, freezer,
+                width, height, run_update, extent,
             ))
 
     return results
 
 
 def _render_size(plugin_id, manifest, plugin_dir, config, mock_data,
-                 width, height, run_update, extent,
-                 cache_manager=None, freezer=None) -> List[RenderResult]:
+                 width, height, run_update, extent) -> List[RenderResult]:
     """Render every mode at one size. A fresh instance per mode avoids state leaks."""
     results: List[RenderResult] = []
 
     # Discover modes once per size (instance build can depend on config).
     try:
         probe_dm = BoundsCheckingDisplayManager(width=width, height=height, overflow_extent=extent)
-        probe = _instantiate(plugin_id, manifest, plugin_dir, config, mock_data, probe_dm,
-                             cache_manager)
+        probe = _instantiate(plugin_id, manifest, plugin_dir, config, mock_data, probe_dm)
         modes = list_modes(probe, manifest, plugin_id)
     except Exception as e:  # noqa: BLE001 — surface any load failure as a result
         return [RenderResult(plugin_id, width, height, "<load>", error=repr(e))]
@@ -353,8 +229,7 @@ def _render_size(plugin_id, manifest, plugin_dir, config, mock_data,
         result = RenderResult(plugin_id, width, height, mode)
         dm = BoundsCheckingDisplayManager(width=width, height=height, overflow_extent=extent)
         try:
-            inst = _instantiate(plugin_id, manifest, plugin_dir, config, mock_data, dm,
-                                cache_manager)
+            inst = _instantiate(plugin_id, manifest, plugin_dir, config, mock_data, dm)
             if run_update:
                 try:
                     inst.update()
@@ -373,9 +248,6 @@ def _render_size(plugin_id, manifest, plugin_dir, config, mock_data,
                 result.display_returned = _render_mode(inst, mode)
                 result.image = dm.get_image()
                 result.overflow = dm.check_overflow()
-                # A blank first frame is not proof of a blank mode; see
-                # _settle_empty_frame.
-                _settle_empty_frame(inst, mode, dm, result, freezer)
         except Exception as e:  # noqa: BLE001 — a display crash is a real failure
             result.error = repr(e)
         results.append(result)

src/plugin_system/testing/loading.py

@@ -33,45 +33,6 @@ def load_manifest(plugin_dir: Union[str, Path]) -> Dict[str, Any]:
         return json.load(f)
 
 
-def _defaults_from_properties(properties: Dict[str, Any]) -> Dict[str, Any]:
-    """Defaults for one `properties` block, recursing into nested objects.
-
-    An object property carries its defaults on its children, not on itself, so
-    reading only the top level dropped everything nested. That is most of the
-    fleet: config organised by league, or under customization/display_options,
-    lost 2,386 defaults across 37 of 44 plugins -- soccer-scoreboard alone lost
-    539 of 565 -- and the harness rendered them with a config no install would
-    ever have.
-    """
-    defaults: Dict[str, Any] = {}
-    for key, prop in (properties or {}).items():
-        if not isinstance(prop, dict):
-            continue
-        if prop.get('type') == 'object' and isinstance(prop.get('properties'), dict):
-            nested = _defaults_from_properties(prop['properties'])
-            if nested:
-                defaults[key] = nested
-        elif 'default' in prop:
-            defaults[key] = prop['default']
-    return defaults
-
-
-def merge_config(base: Dict[str, Any], override: Dict[str, Any]) -> Dict[str, Any]:
-    """Deep-merge override onto base, without dropping sibling defaults.
-
-    A shallow merge would let `-c '{"nhl": {"enabled": true}}'` replace the whole
-    nhl subtree and silently discard every other nhl default -- the same class of
-    bug this function exists to fix.
-    """
-    merged = dict(base)
-    for key, value in (override or {}).items():
-        if isinstance(value, dict) and isinstance(merged.get(key), dict):
-            merged[key] = merge_config(merged[key], value)
-        else:
-            merged[key] = value
-    return merged
-
-
 def load_config_defaults(plugin_dir: Union[str, Path]) -> Dict[str, Any]:
     """Extract default values from a plugin's config_schema.json (empty if none)."""
     schema_path = Path(plugin_dir) / 'config_schema.json'
@@ -79,7 +40,11 @@ def load_config_defaults(plugin_dir: Union[str, Path]) -> Dict[str, Any]:
         return {}
     with open(schema_path, 'r') as f:
         schema = json.load(f)
-    return _defaults_from_properties(schema.get('properties', {}))
+    defaults: Dict[str, Any] = {}
+    for key, prop in schema.get('properties', {}).items():
+        if isinstance(prop, dict) and 'default' in prop:
+            defaults[key] = prop['default']
+    return defaults
 
 
 def load_harness_spec(plugin_dir: Union[str, Path]) -> Dict[str, Any]:

src/plugin_system/testing/visual_display_manager.py

@@ -506,18 +506,9 @@ class VisualTestDisplayManager:
     # Scrolling state (no-op interface compat)
     # ------------------------------------------------------------------
 
-    def set_scrolling_state(self, is_scrolling: bool, frame_hold: int = 1):
-        """Set the current scrolling state (no-op for testing).
-
-        ``frame_hold`` mirrors the DisplayManager signature this change adds.
-        The two are kept in step deliberately: a double that accepts arguments
-        production does not lets a call pass every harness run and then raise
-        TypeError on the panel, and a double that lacks one production has
-        fails every render of a plugin that legitimately paces its scroll.
-        Plugins begin passing it in ledmatrix-plugins#462.
-        """
+    def set_scrolling_state(self, is_scrolling: bool):
+        """Set the current scrolling state (no-op for testing)."""
         self._scrolling_state['is_scrolling'] = is_scrolling
-        self._scrolling_state['frame_hold'] = frame_hold
         if is_scrolling:
             self._scrolling_state['last_scroll_activity'] = time.time()
 

test/test_app_plugins_dir_resolution.py

@@ -0,0 +1,52 @@
+"""web_interface/app.py resolves plugins_dir from config at import time.
+
+`project_root` used to be assigned only inside the `else` branch (the
+relative-path case). SchemaManager and composer_bp are wired up with
+`project_root` further down the same module, so a device configured with an
+*absolute* `plugin_system.plugins_directory` (a supported value -- see
+CLAUDE.md) hit `UnboundLocalError` importing web_interface.app at all, taking
+the whole web UI down.
+
+app.py isn't imported directly by the test suite -- it has real side effects
+(managers, background state) at import time -- so this extracts the exact
+resolution block by its stable comment markers and executes it in isolation,
+the same way the rest of the suite validates generated/templated code rather
+than re-implementing it.
+"""
+from pathlib import Path
+
+APP_PY = Path(__file__).resolve().parent.parent / "web_interface/app.py"
+
+START_MARKER = "# Resolve plugin directory - handle both absolute and relative paths\n"
+END_MARKER = "\nplugin_manager = PluginManager("
+
+
+def _resolution_block() -> str:
+    src = APP_PY.read_text()
+    start = src.index(START_MARKER)
+    end = src.index(END_MARKER, start)
+    return src[start:end]
+
+
+def test_resolution_block_still_matches_expected_markers():
+    """If app.py is restructured enough that these markers move, the exec
+    below would silently test nothing -- fail loudly instead."""
+    block = _resolution_block()
+    assert "plugins_dir" in block and "os.path.isabs" in block
+
+
+def test_absolute_plugins_directory_does_not_raise_unboundlocalerror():
+    import os
+    ns = {"os": os, "Path": Path, "__file__": str(APP_PY),
+          "plugins_dir_name": "/opt/ledmatrix-plugins"}
+    exec(compile(_resolution_block(), str(APP_PY), "exec"), ns)  # noqa: S102
+    assert ns["project_root"] == APP_PY.parent.parent
+    assert ns["plugins_dir"] == Path("/opt/ledmatrix-plugins")
+
+
+def test_relative_plugins_directory_still_resolves_under_project_root():
+    import os
+    ns = {"os": os, "Path": Path, "__file__": str(APP_PY),
+          "plugins_dir_name": "plugin-repos"}
+    exec(compile(_resolution_block(), str(APP_PY), "exec"), ns)  # noqa: S102
+    assert ns["plugins_dir"] == ns["project_root"] / "plugin-repos"

test/test_cache_nonfinite_floats.py

@@ -1,158 +0,0 @@
-"""Non-finite floats round-trip identically with and without orjson.
-
-JSON has no NaN or Infinity. The stdlib emits them anyway as an extension;
-orjson refuses to and writes null. Cache files outlive the decision of which
-encoder is installed, so both halves of that gap are pinned here:
-
-  * writing -- installing orjson must not silently change what gets cached,
-    so the stdlib path writes null too;
-  * reading -- records already on disk carrying NaN or Infinity must stay
-    readable, or installing orjson turns each of them into a "corrupted cache
-    file" that DiskCache.get logs as an error and deletes.
-"""
-
-import importlib
-import json
-import math
-import sys
-from pathlib import Path
-from unittest import mock
-
-import pytest
-
-sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
-
-from src.cache import disk_cache as disk_cache_module  # noqa: E402
-
-
-@pytest.fixture
-def stdlib_cache():
-    """The module as it loads on a host with no orjson wheel."""
-    # A None entry in sys.modules makes `import orjson` raise ImportError,
-    # which is the branch we want, whether or not orjson is really installed.
-    with mock.patch.dict(sys.modules, {"orjson": None}):
-        module = importlib.reload(disk_cache_module)
-        assert module.orjson is None
-        yield module
-    importlib.reload(disk_cache_module)
-
-
-@pytest.fixture
-def orjson_cache():
-    """The module as it loads with orjson available."""
-    module = importlib.reload(disk_cache_module)
-    if module.orjson is None:
-        pytest.skip("orjson is not installed on this host")
-    return module
-
-
-NON_FINITE = {"nan": float("nan"), "inf": float("inf"), "ninf": float("-inf")}
-
-
-def _reject_constant(name):
-    """Make json.loads as strict as orjson about NaN/Infinity tokens."""
-    raise AssertionError(f"non-spec JSON constant in output: {name}")
-
-
-class TestWritePolicy:
-    """Non-finite floats become null on whichever encoder is in use."""
-
-    def _assert_nulled(self, module):
-        record = module._loads(module._dumps(dict(NON_FINITE, ok=1.5)))
-        assert record["nan"] is None
-        assert record["inf"] is None
-        assert record["ninf"] is None
-        # Finite values are untouched.
-        assert record["ok"] == 1.5
-
-    def test_stdlib_writes_null(self, stdlib_cache):
-        self._assert_nulled(stdlib_cache)
-
-    def test_orjson_writes_null(self, orjson_cache):
-        self._assert_nulled(orjson_cache)
-
-    def test_stdlib_emits_spec_compliant_json(self, stdlib_cache):
-        # The point of the write half: bytes written without orjson must still
-        # parse once orjson is installed later. json.loads accepts the
-        # extension tokens, so it cannot show this -- assert on the bytes, and
-        # on a strict reader when there is one.
-        raw = stdlib_cache._dumps(dict(NON_FINITE))
-        assert b"NaN" not in raw
-        assert b"Infinity" not in raw
-        assert json.loads(raw, parse_constant=_reject_constant) == {
-            "nan": None, "inf": None, "ninf": None}
-
-    def test_nested_non_finite_are_replaced(self, stdlib_cache):
-        data = {"a": [1.0, float("nan"), {"b": float("inf")}], "c": (float("-inf"),)}
-        record = stdlib_cache._loads(stdlib_cache._dumps(data))
-        assert record["a"] == [1.0, None, {"b": None}]
-        assert record["c"] == [None]
-
-    def test_finite_payloads_are_byte_identical_to_the_old_encoder(self, stdlib_cache):
-        # allow_nan=False must not change ordinary output.
-        data = {"x": 1, "y": [1.5, "s", True, None], "z": {"k": 2.25}}
-        assert stdlib_cache._dumps(data) == json.dumps(
-            data, cls=stdlib_cache.DateTimeEncoder).encode("utf-8")
-
-
-class TestReplaceNonFinite:
-    def test_leaves_ordinary_values_alone(self):
-        for value in (1, 1.5, "s", True, None, [], {}):
-            assert disk_cache_module._replace_nonfinite(value) == value
-
-    def test_replaces_every_non_finite_float(self):
-        for value in NON_FINITE.values():
-            assert disk_cache_module._replace_nonfinite(value) is None
-
-    def test_walks_nested_containers(self):
-        assert disk_cache_module._replace_nonfinite(
-            {"a": [{"b": float("nan")}]}) == {"a": [{"b": None}]}
-
-
-class TestLegacyRecordsStayReadable:
-    """Files written before orjson arrived still load."""
-
-    LEGACY = b'{"timestamp": 1000.0, "value": NaN, "other": Infinity}'
-
-    def test_stdlib_reads_legacy_tokens(self, stdlib_cache):
-        record = stdlib_cache._loads(self.LEGACY)
-        assert math.isnan(record["value"])
-        assert math.isinf(record["other"])
-
-    def test_orjson_falls_back_for_legacy_tokens(self, orjson_cache):
-        record = orjson_cache._loads(self.LEGACY)
-        assert math.isnan(record["value"])
-        assert math.isinf(record["other"])
-
-    def test_genuinely_malformed_files_still_raise(self, stdlib_cache):
-        with pytest.raises(json.JSONDecodeError):
-            stdlib_cache._loads(b'{"a": ')
-
-    def test_orjson_still_raises_for_malformed_files(self, orjson_cache):
-        with pytest.raises(json.JSONDecodeError):
-            orjson_cache._loads(b'{"a": ')
-
-
-class TestDiskCacheEndToEnd:
-    def _cache(self, module, tmp_path):
-        return module.DiskCache(str(tmp_path))
-
-    def test_legacy_file_is_not_deleted_as_corrupt(self, orjson_cache, tmp_path):
-        cache = self._cache(orjson_cache, tmp_path)
-        path = cache.get_cache_path("legacy")
-        Path(path).write_bytes(
-            b'{"timestamp": %d, "value": NaN}' % int(__import__("time").time()))
-
-        record = cache.get("legacy", max_age=None)
-
-        assert record is not None, "legacy NaN record was treated as corrupt"
-        assert math.isnan(record["value"])
-        assert Path(path).exists(), "legacy NaN record was deleted"
-
-    def test_round_trip_through_set_and_get(self, stdlib_cache, tmp_path):
-        cache = self._cache(stdlib_cache, tmp_path)
-        cache.set("k", {"timestamp": __import__("time").time(),
-                        "value": float("nan")})
-        record = cache.get("k", max_age=None)
-        assert record is not None
-        assert record["value"] is None

test/test_composer_blueprint_registered.py

@@ -0,0 +1,64 @@
+"""composer_bp must actually be registered on the Flask app.
+
+web_interface/blueprints/composer.py defines a full blueprint -- the
+/composer/ page, /composer/api/generate, /composer/api/install, etc. -- but
+nothing in web_interface/app.py imported or registered it. pages_v3 and
+api_v3 both go through the same "import, wire managers, register_blueprint"
+sequence; composer_bp had none of the three. The routes existed and worked
+in isolation (every other composer test builds its own minimal Flask app
+and registers composer_bp directly), but in the real running app every
+composer URL -- including the page itself -- was a 404, and the frontend's
+hardcoded '/composer/api/...' fetches (composer-app.js) had nothing to
+reach.
+
+app.py has import-time side effects (it loads config, builds managers, and
+in some configurations starts background threads), so this scans the source
+text rather than importing the module -- the same approach already used for
+test_composer_js_contracts.py's structural checks.
+"""
+import re
+from pathlib import Path
+
+APP_PY = Path(__file__).resolve().parent.parent / "web_interface" / "app.py"
+
+
+def _source() -> str:
+    return APP_PY.read_text()
+
+
+def test_composer_blueprint_is_imported():
+    src = _source()
+    assert re.search(
+        r"from web_interface\.blueprints\.composer import composer_bp", src
+    ), "app.py never imports composer_bp"
+
+
+def test_composer_blueprint_is_registered_under_composer_prefix():
+    src = _source()
+    match = re.search(
+        r"app\.register_blueprint\(\s*composer_bp\s*,\s*url_prefix\s*=\s*"
+        r"['\"](?P<prefix>[^'\"]*)['\"]",
+        src,
+    )
+    assert match, "app.py never calls app.register_blueprint(composer_bp, ...)"
+    # composer.html and composer-app.js hardcode '/composer/api/...' and
+    # '/composer/api/fonts/...' -- any other prefix serves a page whose own
+    # fetches and @font-face all 404.
+    assert match.group("prefix") == "/composer"
+
+
+def test_composer_blueprint_managers_are_wired_before_registration():
+    """Registered with no managers set means every route 503s or crashes.
+
+    composer.py initialises config_manager/plugin_manager/plugins_dir/
+    project_root to None at import time specifically so app.py can set them
+    -- the same pattern api_v3 and pages_v3 use.
+    """
+    src = _source()
+    register_match = re.search(r"app\.register_blueprint\(\s*composer_bp", src)
+    assert register_match, "app.py never registers composer_bp"
+    before_registration = src[: register_match.start()]
+    for attr in ("config_manager", "plugin_manager", "plugins_dir", "project_root"):
+        assert re.search(rf"composer_bp\.{attr}\s*=", before_registration), (
+            f"composer_bp.{attr} is never set before registration"
+        )

test/test_composer_code_injection.py

@@ -0,0 +1,323 @@
+"""The composer generates Python that the plugin loader imports and executes.
+
+/api/install writes the generated manager.py into plugins_dir and the loader
+imports it, so anything the payload can splice into that source runs on the
+device. The ast.parse check in _generate_plugin_files rejects only *invalid*
+syntax -- an injected `import os` is perfectly valid and passed it.
+
+Two ways in, both confirmed against the code before it was fixed:
+
+  metadata.name = a name containing a triple-quote, a newline, then
+                  `import os; PWNED = os.getuid()`, then another triple-quote
+      -> closes the module docstring; the rest became module-level statements
+         (spelled out rather than shown literally -- writing the payload into
+         this docstring closes *this* file's docstring, which is the bug)
+
+  element x     = '0 or __import__("os").system("id")'
+      -> f-string interpolated it verbatim: x=0 or __import__("os").system("id")
+"""
+import ast
+import re
+import sys
+from pathlib import Path
+
+import pytest
+
+sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
+
+from web_interface.blueprints import composer as C  # noqa: E402
+
+BASE_META = {"id": "test-plugin", "name": "Clock", "author": "a",
+             "version": "1.0.0", "description": "d"}
+
+#: Values that terminate a Python expression and start a new statement.
+EXPR_PAYLOADS = [
+    '0 or __import__("os").system("id")',
+    '0);import os;os.system("id");(',
+    '__import__("subprocess").run(["id"])',
+    "0 if False else exec('x=1')",
+    "1e999", "nan", "0x41", "0__0",
+]
+
+#: Values that close a string literal in the generated source.
+LITERAL_PAYLOADS = [
+    'Clock"""\nimport os; PWNED = os.getuid()\n"""',
+    "Clock'''\nimport os\n'''",
+    'Clock" + __import__("os").system("id") + "',
+    "Clock\\", "Clock\nimport os",
+]
+
+
+def _payload(**over):
+    # dataModel.configVars is the key _generate_plugin_files reads; "config_vars"
+    # was never looked at, so anything passed through it tested nothing.
+    p = {"metadata": dict(BASE_META), "elements": [],
+         "dataModel": {"configVars": over.pop("config_vars", [])}}
+    p["metadata"].update(over.pop("metadata", {}))
+    p.update(over)
+    return p
+
+
+def _generated(payload):
+    return C._generate_plugin_files(payload)["manager.py"]
+
+
+def _module_level_code(src):
+    """Statements at module level that are not the docstring/imports/classes."""
+    tree = ast.parse(src)
+    out = []
+    for node in tree.body:
+        if isinstance(node, (ast.ClassDef, ast.FunctionDef, ast.ImportFrom, ast.Import)):
+            continue
+        if isinstance(node, ast.Expr) and isinstance(node.value, ast.Constant):
+            continue  # the docstring
+        out.append(ast.unparse(node))
+    return out
+
+
+@pytest.mark.parametrize("payload", LITERAL_PAYLOADS)
+def test_a_name_that_breaks_out_of_a_literal_is_refused(payload):
+    with pytest.raises(C.ComposerInputError):
+        _generated(_payload(metadata={"name": payload}))
+
+
+#: Types with a drawing branch in manager.py.j2. An injection test using any
+#: other type proves nothing: _preprocess_elements drops it, so its values
+#: never reach the generated source and every assertion passes trivially.
+#: This test previously used "line", which has never had a branch.
+RENDERED_GEOMETRY_CASES = [
+    ("rectangle", {"x": 0, "y": 0, "width": 10, "height": 8}),
+    ("arc", {"x": 0, "y": 0, "width": 24, "height": 24}),
+    ("ellipse", {"x": 0, "y": 0, "width": 24, "height": 12}),
+    ("rounded_rectangle", {"x": 0, "y": 0, "width": 24, "height": 10}),
+    ("gauge", {"x": 0, "y": 0, "width": 32, "height": 32}),
+]
+
+
+@pytest.mark.parametrize("etype,base", RENDERED_GEOMETRY_CASES)
+@pytest.mark.parametrize("evil", EXPR_PAYLOADS)
+@pytest.mark.parametrize("field", ["x", "y", "width", "height"])
+def test_a_non_numeric_geometry_value_cannot_reach_the_source(etype, base, evil, field):
+    """width/height were interpolated raw into the generated source.
+
+    p['x2_expr'] = f"({x_expr}) + {w}" with w straight off the payload, so a
+    rectangle with width='0 or __import__("os").system("id")' produced
+
+        [0, 0, (0) + 0 or __import__("os").system("id"), (0) + 8],
+
+    in a manager.py that /api/install writes to disk and the loader imports.
+    """
+    el = {"type": etype, "id": "e1", **base}
+    el[field] = evil
+    src = _generated(_payload(elements=[el]))
+    assert "__import__" not in src, f"{etype}.{field}={evil!r} reached the generated source"
+    assert "os.system" not in src
+    assert not _module_level_code(src), \
+        f"{etype}.{field}={evil!r} produced module-level statements: {_module_level_code(src)}"
+
+
+def test_every_injection_case_uses_a_type_that_actually_renders():
+    """Guards against the whole suite quietly going vacuous again.
+
+    An element type with no template branch is dropped before generation, so
+    an injection test written against one asserts nothing and still passes.
+    """
+    used = {etype for etype, _ in RENDERED_GEOMETRY_CASES}
+    missing = used - set(C._RENDERABLE_ELEMENT_TYPES)
+    assert not missing, f"injection tests use non-rendering types: {sorted(missing)}"
+
+
+@pytest.mark.parametrize("evil", EXPR_PAYLOADS)
+@pytest.mark.parametrize("channel", ["r", "g", "b"])
+def test_a_non_numeric_colour_channel_cannot_reach_the_source(evil, channel):
+    el = {"type": "text", "id": "t1", "x": 0, "y": 0, "text": "hi",
+          "font": "press_start", "r": 255, "g": 255, "b": 255}
+    el[channel] = evil
+    src = _generated(_payload(elements=[el]))
+    assert "__import__" not in src and "os.system" not in src
+    assert not _module_level_code(src)
+
+
+def test_colour_channels_are_clamped_to_a_byte():
+    el = {"type": "text", "id": "t1", "x": 0, "y": 0, "text": "hi",
+          "font": "press_start", "r": 99999, "g": -5, "b": 128}
+    src = _generated(_payload(elements=[el]))
+    assert "(255, 0, 128)" in src, "channels were not clamped to 0-255"
+
+
+#: fillR/fillG/fillB (and outR/G/B, bgR/G/B, trackR/G/B) go through
+#: _as_fill_filter -> _as_rgb_filter, a separate path from _rgb_expr above.
+#: It used raw int() until it was found to raise ValueError on a non-numeric
+#: channel instead of clamping like every other coerced value in this module.
+@pytest.mark.parametrize("evil", EXPR_PAYLOADS)
+@pytest.mark.parametrize("channel", ["fillR", "fillG", "fillB"])
+def test_a_non_numeric_fill_channel_cannot_reach_the_source(evil, channel):
+    el = {"type": "rectangle", "id": "r1", "x": 0, "y": 0, "width": 10, "height": 8,
+          "fillR": 0, "fillG": 0, "fillB": 128}
+    el[channel] = evil
+    src = _generated(_payload(elements=[el]))
+    assert "__import__" not in src and "os.system" not in src
+    assert not _module_level_code(src)
+
+
+def test_fill_channels_are_clamped_to_a_byte():
+    el = {"type": "rectangle", "id": "r1", "x": 0, "y": 0, "width": 10, "height": 8,
+          "fillR": 99999, "fillG": -5, "fillB": 128}
+    src = _generated(_payload(elements=[el]))
+    assert "(255, 0, 128)" in src, "fill channels were not clamped to 0-255"
+
+
+def test_the_generated_module_still_has_no_top_level_statements():
+    """The clean case: a normal payload produces only imports and a class."""
+    el = {"type": "text", "id": "t1", "x": 4, "y": 4, "text": "hi",
+          "font": "press_start", "r": 1, "g": 2, "b": 3}
+    src = _generated(_payload(elements=[el]))
+    assert not _module_level_code(src)
+    assert "(1, 2, 3)" in src
+
+
+# --- config variable keys ---------------------------------------------------
+
+def _with_key(key):
+    return {"metadata": dict(BASE_META), "elements": [],
+            "dataModel": {"configVars": [{"key": key, "type": "string",
+                                          "default": "x", "label": "L"}]}}
+
+
+@pytest.mark.parametrize("key", ["class", "def", "import", "None", "True",
+                                 "lambda", "pass", "match", "case"])
+def test_a_keyword_config_key_is_named_in_the_error(key):
+    """ast.parse already rejected these, but as an unhelpful line number.
+
+    "Generated code has a syntax error: invalid syntax (line 17)" tells the
+    user nothing about which field to fix.
+    """
+    with pytest.raises(C.ComposerInputError) as exc:
+        _generated(_with_key(key))
+    assert key in str(exc.value) and "keyword" in str(exc.value).lower()
+
+
+@pytest.mark.parametrize("key", ["config", "logger", "display_manager",
+                                 "cache_manager", "plugin_id", "enabled",
+                                 "self", "update", "display"])
+def test_a_reserved_attribute_config_key_is_refused(key):
+    """These generate *valid* Python that silently clobbers plugin state.
+
+    The worst is `config`: the assignment lands right after super().__init__(),
+    so `self.config = config.get("config", "x")` replaces the plugin's config
+    dict with a string and every later self.config.get(...) fails at runtime.
+    """
+    with pytest.raises(C.ComposerInputError) as exc:
+        _generated(_with_key(key))
+    assert key in str(exc.value) and "reserved" in str(exc.value).lower()
+
+
+@pytest.mark.parametrize("key", ["brightness", "my_var", "_private", "x1",
+                                 "update_interval_seconds"])
+def test_ordinary_config_keys_are_still_accepted(key):
+    src = _generated(_with_key(key))
+    assert f"self.{key} = config.get(" in src
+
+
+def test_the_generated_config_assignment_does_not_precede_super_init():
+    """Guards the reasoning behind the reserved list, not just the list."""
+    src = _generated(_with_key("brightness"))
+    body = src.splitlines()
+    super_at = next(i for i, line in enumerate(body) if "super().__init__(" in line)
+    assign_at = next(i for i, line in enumerate(body)
+                     if "self.brightness = config.get(" in line)
+    assert assign_at > super_at, (
+        "config vars are assigned before super().__init__(); the reserved-name "
+        "list assumes they land after it")
+
+
+# --- optional keys ----------------------------------------------------------
+
+@pytest.mark.parametrize("el_type,missing", [
+    ("text", "text"), ("text", "text2"), ("clock", "format"),
+])
+def test_an_element_missing_an_optional_key_does_not_500(el_type, missing):
+    """`p` is a copy of the raw element, so an absent key stays absent.
+
+    The defaults were applied to locals only, so manager.py.j2 rendered
+    `{{ el.text | tojson }}` over a jinja2.Undefined and tojson raised
+    TypeError -- which no handler catches, making a missing key a 500 rather
+    than a validation error or a sensible default.
+    """
+    el = {"type": el_type, "id": "e1", "x": 0, "y": 0, "font": "press_start"}
+    src = _generated(_payload(elements=[el]))
+    ast.parse(src)          # must still be valid Python
+    assert "Undefined" not in src
+
+
+def test_a_clock_without_a_format_uses_the_documented_default():
+    el = {"type": "clock", "id": "c1", "x": 0, "y": 0, "font": "press_start"}
+    src = _generated(_payload(elements=[el]))
+    assert '"%H:%M"' in src, "the %H:%M default did not reach the generated source"
+
+
+#: (element type, channel key, base element) for colour channels that were
+#: interpolated raw rather than through _rgb_expr/_safe_int. Prefixed channels
+#: (emptyR/G/B, labelR/G/B) were the ones the original r/g/b test never reached.
+RAW_COLOUR_CASES = [
+    ("progress_bar", "r",      {"x": 0, "y": 0}),
+    ("progress_bar", "g",      {"x": 0, "y": 0}),
+    ("pips",         "b",      {"x": 0, "y": 0}),
+    ("pips",         "emptyR", {"x": 0, "y": 0}),
+    ("pips",         "emptyG", {"x": 0, "y": 0}),
+    ("sparkline",    "r",      {"x": 0, "y": 0}),
+    ("gauge",        "labelR", {"x": 0, "y": 0, "width": 32, "height": 32}),
+    ("gauge",        "labelB", {"x": 0, "y": 0, "width": 32, "height": 32}),
+]
+
+
+@pytest.mark.parametrize("etype,channel,base", RAW_COLOUR_CASES)
+@pytest.mark.parametrize("evil", EXPR_PAYLOADS)
+def test_a_prefixed_colour_channel_cannot_reach_the_source(etype, channel, base, evil):
+    """Five tuples were built with f"({el.get('r', 100)}, ...)" -- no coercion.
+
+    The pre-existing colour test only covered r/g/b on a text element, so the
+    prefixed channels and the four other types were never exercised.
+    """
+    el = {"type": etype, "id": "e1", **base}
+    el[channel] = evil
+    src = _generated(_payload(elements=[el]))
+    assert "__import__" not in src, f"{etype}.{channel}={evil!r} reached the source"
+    assert "os.system" not in src
+    assert not _module_level_code(src)
+
+
+@pytest.mark.parametrize("value", [float("inf"), float("-inf"), float("nan")])
+@pytest.mark.parametrize("field", ["x", "y", "width", "height"])
+def test_a_non_finite_dimension_does_not_escape_as_an_unhandled_error(field, value):
+    """json.loads accepts Infinity/NaN and Flask passes them through, so a
+    payload can hand _safe_int a non-finite float. int(inf) raises
+    OverflowError -- neither ValueError nor ComposerInputError -- so it escaped
+    both handlers and surfaced as a 500 with a traceback instead of a 422."""
+    el = {"type": "rectangle", "id": "r1", "x": 0, "y": 0, "width": 10, "height": 8}
+    el[field] = value
+    src = _generated(_payload(elements=[el]))       # must not raise
+    # A non-finite value must be replaced by the default, not spelled into the
+    # source. Word-boundary match: "info" in self.logger.info contains "inf".
+    assert not re.search(r"\b(inf|nan|Infinity|NaN)\b", src), \
+        f"{field}={value!r} leaked a non-finite literal into the source"
+    assert not _module_level_code(src)
+
+
+@pytest.mark.parametrize("bad_id", [
+    'x = __import__("os").system("id") #',
+    "x\nimport os\n_y",
+    "x[0]",
+    "",
+    "a" * 200,
+])
+def test_a_marquee_id_cannot_become_code(bad_id):
+    """data_key is spliced UNQUOTED into variable names
+    (_{{ data_key }}_text = ...), so a non-identifier id landed in the source
+    as code. ast.parse caught it, but the caller then got an opaque
+    "Generated code has a syntax error" rather than being told the id is bad."""
+    el = {"type": "marquee", "id": bad_id, "x": 0, "y": 0, "text": "hi"}
+    src = _generated(_payload(elements=[el]))       # must not raise
+    assert "__import__(" not in src
+    assert "os.system(" not in src
+    assert not _module_level_code(src)

test/test_composer_empty_block.py

@@ -0,0 +1,124 @@
+"""An element the template cannot draw must not produce an empty `if` block.
+
+manager.py.j2 wraps each element in `if width >= N:` (breakpoint) and/or
+`if int(time.time() * 2) % 2:` (blink), and the body comes from the per-type
+branches. A type with no branch contributed nothing, so the wrapper opened a
+block with no statements in it. ast.parse in _generate_plugin_files then
+failed and the caller was told only:
+
+    Generated code has a syntax error: expected an indented block after
+    'if' statement on line 49
+
+which names a line of generated source the user never sees. Confirmed against
+the code before the fix with a `group` element carrying minWidth.
+
+Two defences, both covered here: _preprocess_elements drops types the template
+has no branch for, and the template emits a `pass` fallback so a type added to
+the canvas before its branch exists degrades to a no-op instead of a broken
+plugin.
+"""
+import ast
+import re
+import sys
+from pathlib import Path
+
+import pytest
+
+sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
+
+from web_interface.blueprints import composer as C  # noqa: E402
+
+TEMPLATE = (Path(__file__).resolve().parent.parent
+            / "web_interface/templates/v3/composer/manager.py.j2")
+
+BASE_META = {"id": "test-plugin", "name": "Clock", "author": "a",
+             "version": "1.0.0", "description": "d"}
+
+
+def generate(element):
+    return C._generate_plugin_files({
+        "metadata": BASE_META,
+        "elements": [element],
+        "dataModel": {"configVars": []},
+    })
+
+
+@pytest.mark.parametrize("wrapper", [
+    {"minWidth": 64},          # breakpoint block
+    {"blink": True},           # blink block
+    {"minWidth": 64, "blink": True},   # both, nested
+])
+@pytest.mark.parametrize("etype", ["group", "widget_9000", "section"])
+def test_undrawable_element_does_not_break_generation(etype, wrapper):
+    element = {"type": etype, "x": 0, "y": 0, "color": "#ffffff", **wrapper}
+    files = generate(element)          # must not raise ComposerInputError
+    assert "manager.py" in files
+
+
+def test_drawable_element_still_renders_inside_a_breakpoint():
+    files = generate({"type": "text", "text": "hi", "x": 0, "y": 0,
+                      "minWidth": 64, "color": "#ffffff"})
+    src = files["manager.py"]
+    assert "if width >= 64:" in src
+    assert "draw_text" in src
+
+
+def test_renderable_types_match_the_template_branches():
+    """The constant and the template must agree.
+
+    A type listed in the constant with no branch emits an empty block (the bug
+    above); a type with a branch but missing from the constant is silently
+    dropped from every generated plugin. Neither is visible without this check.
+    """
+    branches = set(re.findall(r"el\.type == '([a-z_]+)'", TEMPLATE.read_text()))
+    assert branches == set(C._RENDERABLE_ELEMENT_TYPES)
+
+
+def test_template_closes_the_branch_chain_with_a_fallback():
+    """Belt and braces: even if the constant drifts, no empty block escapes."""
+    text = TEMPLATE.read_text()
+    assert "{% else %}" in text
+    assert "pass  # element type" in text
+
+
+@pytest.mark.parametrize("wrapper", [
+    {"minWidth": 64},
+    {"blink": True},
+    {"minWidth": 64, "blink": True},
+])
+def test_dynamic_text_with_non_config_binding_does_not_break_generation(wrapper):
+    """dynamic_text only renders a body for binding.source == 'config'.
+
+    _preprocess_elements accepts any string as binding.source (it just
+    defaults a missing one to 'config'), so a 'live' or 'sensor' source --
+    anything a client sends that isn't literally 'config' -- hit the same
+    empty-if-block bug as an undrawable element type, just one level deeper:
+    the branch is taken, but its own inner `if` produced nothing.
+    """
+    element = {"type": "dynamic_text", "x": 0, "y": 0, "color": "#ffffff",
+               "binding": {"source": "live", "key": "temperature"}, **wrapper}
+    files = generate(element)          # must not raise ComposerInputError
+    assert "manager.py" in files
+    assert "non-config dynamic_text binding draws nothing" in files["manager.py"]
+
+
+def test_binding_source_cannot_break_out_of_the_comment_it_lands_in():
+    """binding.source used to be interpolated straight into a Python comment
+    (`pass  # dynamic_text binding_source "{{ el.binding_source }}" draws
+    nothing`) with no escaping. A source string carrying a newline closed the
+    comment, and text on the following line(s), indented to match, became a
+    real statement in the generated plugin -- CWE-94, and live once
+    composer_bp was registered (confirmed: this exact payload produces a
+    manager.py containing a clean, ast.parse-valid `import os` against the
+    pre-fix template). The comment is now a fixed literal that never
+    interpolates the value at all.
+    """
+    payload = "foo\n            import os\n            os.system('id')  # "
+    element = {"type": "dynamic_text", "x": 0, "y": 0, "color": "#ffffff",
+               "binding": {"source": payload, "key": "temperature"}}
+    files = generate(element)
+    src = files["manager.py"]
+    assert "import os" not in src
+    assert "os.system" not in src
+    assert "non-config dynamic_text binding draws nothing" in src
+    ast.parse(src)          # belt and braces: generate() already enforces this

test/test_composer_js_contracts.py

@@ -0,0 +1,263 @@
+"""Structural checks on the composer's JavaScript.
+
+There is no JS test runner in this repo, but three defects here are structural
+enough to assert on the parse tree, and each was a real bug:
+
+- Stroke widths inside _drawElement were left in canvas pixels while the
+  geometry around them scaled by SCALE, so at SCALE>1 every outline rendered
+  thinner than one LED pixel and the preview stopped matching the panel.
+- The `line` branch drew raw el.x0/el.y0/el.x1/el.y1, ignoring the anchor that
+  every other element type honours, so setting xAnchor moved everything except
+  lines -- and getBoundingBox had the same omission, leaving the hit box behind.
+- Four methods mutated exactly what _snapshot() serialises (metadata,
+  currentPreset) without calling it. _snapshot is the only caller of
+  _debouncedAutosave, so those changes were lost on reload and could not be
+  undone.
+"""
+import re
+from pathlib import Path
+
+import pytest
+
+tree_sitter = pytest.importorskip("tree_sitter")
+tree_sitter_javascript = pytest.importorskip("tree_sitter_javascript")
+
+JS_DIR = Path(__file__).resolve().parent.parent / "web_interface/static/v3/js/composer"
+CANVAS = JS_DIR / "composer-canvas.js"
+APP = JS_DIR / "composer-app.js"
+
+
+def _function_source(path: Path, name: str) -> str:
+    """Return the source of a top-level function declaration by name."""
+    src = path.read_bytes()
+    lang = tree_sitter.Language(tree_sitter_javascript.language())
+    tree = tree_sitter.Parser(lang).parse(src)
+    found = []
+
+    def walk(node):
+        if node.type == "function_declaration":
+            ident = node.child_by_field_name("name")
+            if ident is not None and src[ident.start_byte:ident.end_byte].decode() == name:
+                found.append(src[node.start_byte:node.end_byte].decode())
+        for c in node.children:
+            walk(c)
+
+    walk(tree.root_node)
+    assert found, f"{name} not found in {path.name}"
+    return found[0]
+
+
+def _method_source(path: Path, name: str) -> str:
+    """Return the source of a top-level object method by name."""
+    src = path.read_bytes()
+    lang = tree_sitter.Language(tree_sitter_javascript.language())
+    tree = tree_sitter.Parser(lang).parse(src)
+    found = []
+
+    def walk(node):
+        if node.type == "method_definition":
+            ident = node.child_by_field_name("name")
+            if ident is not None and src[ident.start_byte:ident.end_byte].decode() == name:
+                found.append(src[node.start_byte:node.end_byte].decode())
+        for c in node.children:
+            walk(c)
+
+    walk(tree.root_node)
+    assert found, f"{name} not found in {path.name}"
+    return found[0]
+
+
+def test_both_files_parse():
+    lang = tree_sitter.Language(tree_sitter_javascript.language())
+    parser = tree_sitter.Parser(lang)
+    for path in (CANVAS, APP):
+        tree = parser.parse(path.read_bytes())
+        errors = []
+
+        def walk(node):
+            if node.type == "ERROR" or node.is_missing:
+                errors.append(node.start_point[0] + 1)
+            for c in node.children:
+                walk(c)
+
+        walk(tree.root_node)
+        assert not errors, f"{path.name} has parse errors at lines {errors}"
+
+
+def test_element_strokes_scale_with_scale():
+    """No bare `ctx.lineWidth = 1` inside _drawElement.
+
+    Selection handles and the grid are drawn in canvas pixels deliberately and
+    live in other functions, so this is scoped to the element drawing routine.
+    """
+    body = _function_source(CANVAS, "_drawElement")
+    offenders = re.findall(r"ctx\.lineWidth\s*=\s*1\s*;", body)
+    assert not offenders, f"{len(offenders)} unscaled stroke width(s) in _drawElement"
+
+
+def test_line_branch_applies_the_anchor_offset():
+    """Scoped to _drawElement.
+
+    getBoundingBox has its own `case 'line': {` and appears first in the file,
+    so searching the whole text found *that* branch -- this assertion passed
+    with the draw branch's anchor offset removed. Verified: stripping it and
+    re-running gave 11/11 green.
+    """
+    body = _function_source(CANVAS, "_drawElement")
+    line_branch = body[body.index("case 'line': {"):]
+    line_branch = line_branch[:line_branch.index("case 'divider'")]
+    assert "ax - el.x0" in line_branch and "ay - el.y0" in line_branch, \
+        "line drawing ignores xAnchor/yAnchor"
+    assert "moveTo(el.x0 * s" not in line_branch, \
+        "line still drawn from unanchored endpoints"
+
+
+def test_line_bounding_box_applies_the_anchor_offset():
+    """The companion to the above: scoped to getBoundingBox specifically, so
+    the two tests cannot both be satisfied by the same branch."""
+    body = _function_source(CANVAS, "getBoundingBox")
+    box = body[body.index("case 'line'"):]
+    box = box[:box.index("case 'divider'")]
+    assert "ax - el.x0" in box, "line bounding box ignores the anchor"
+
+
+@pytest.mark.parametrize("method", [
+    "onBgColorChange",     # mutates metadata.bgColor
+    "setCustomSize",       # mutates currentPreset / MATRIX_W / MATRIX_H
+    "changePreset",        # mutates currentPreset / MATRIX_W / MATRIX_H
+    "applyPresetLabel",    # same, for sizes not in DISPLAY_PRESETS
+    "onColorChange",       # the one that was already fixed — keeps it fixed
+])
+def test_state_mutations_take_a_snapshot(method):
+    body = _method_source(APP, method)
+    assert "_snapshot()" in body, \
+        f"{method} changes snapshotted state without calling _snapshot()"
+    assert "isDirty = true" in body, f"{method} does not mark the design dirty"
+
+
+@pytest.mark.parametrize("method", ["changePreset", "applyPresetLabel"])
+def test_restore_path_stays_snapshot_free(method):
+    """_applyState and loadTemplate call these with {silent: true} while
+    restoring; snapshotting there would push restore steps onto the undo stack
+    and re-autosave the state just loaded."""
+    body = _method_source(APP, method)
+    assert "opts.silent" in body, f"{method} lost its silent guard"
+    snap = body.index("_snapshot()")
+    guard = body.index("!opts.silent")
+    assert guard < snap, f"{method} snapshots outside the !opts.silent guard"
+
+
+TEMPLATE_HTML = (Path(__file__).resolve().parent.parent
+                 / "web_interface/templates/v3/composer.html")
+
+#: The six toolbar buttons and the wrapper each must call.
+ALIGN_BUTTONS = ["alignLeft", "alignHCenter", "alignRight",
+                 "alignTop", "alignVCenter", "alignBottom"]
+
+
+def test_alignment_buttons_use_the_anchor_clearing_path():
+    """Two alignment implementations existed and the toolbar used the wrong one.
+
+    The legacy alignElement(dir) set el.x/el.y but left xAnchor/yAnchor in
+    place. resolveAnchor turns anchor='right' into `dim - val`, so "align left"
+    (el.x = 0) resolved to x = MATRIX_W -- the element jumped to the far right
+    edge instead. _alignElement clears the anchor first, so the stored value is
+    absolute, and it also updates el.x0/el.y0 so lines actually move.
+    """
+    html = TEMPLATE_HTML.read_text()
+    for wrapper in ALIGN_BUTTONS:
+        assert f"{wrapper}()" in html, f"toolbar does not call {wrapper}()"
+    assert not re.search(r"[^_]alignElement\(", html), \
+        "toolbar still calls the legacy alignElement()"
+
+
+def test_the_legacy_alignelement_is_gone():
+    """Leaving it in place invites the toolbar drifting back to it."""
+    src = APP.read_text()
+    assert not re.search(r"^\s{4}alignElement\(dir\)", src, re.M), \
+        "legacy alignElement(dir) still defined"
+
+
+def test_align_clears_the_anchor_and_moves_line_endpoints():
+    body = _method_source(APP, "_alignElement")
+    assert "xAnchor = null" in body and "yAnchor = null" in body, \
+        "_alignElement no longer clears the anchor, so aligning an anchored " \
+        "element resolves to the wrong edge"
+    assert "el.x0" in body and "el.y0" in body, \
+        "_alignElement no longer moves line endpoints"
+
+
+def test_align_translates_both_line_endpoints_not_just_the_start():
+    """Setting only x0 (or y0) left x1/y1 behind, so aligning a line changed
+    its shape instead of moving it -- e.g. a line from x0=20 to x1=50 aligned
+    right became x0=98, x1=50, stretching rather than translating it. Both
+    endpoints must move by the same delta."""
+    body = _method_source(APP, "_alignElement")
+    x_branch = body[body.index("if (axis === 'x')"):body.index("} else {")]
+    y_branch = body[body.index("} else {"):]
+    assert "el.x1" in x_branch, \
+        "_alignElement moves x0 but not x1 -- a line's shape changes, not its position"
+    assert "el.y1" in y_branch, \
+        "_alignElement moves y0 but not y1 -- a line's shape changes, not its position"
+
+
+def test_divider_stroke_is_centered_in_led_pixels():
+    """A 0.5 canvas-pixel offset (correct only at SCALE=1) was applied after
+    scaling instead of before it, so at SCALE>1 the stroke bled into the
+    preceding LED row/column instead of straddling its own."""
+    body = _function_source(CANVAS, "_drawElement")
+    divider_branch = body[body.index("case 'divider'"):]
+    divider_branch = divider_branch[:divider_branch.index("case 'pips'")]
+    assert "ay * s + 0.5" not in divider_branch, \
+        "divider still offsets by 0.5 canvas pixels after scaling"
+    assert "ax * s + 0.5" not in divider_branch, \
+        "divider still offsets by 0.5 canvas pixels after scaling"
+    assert "(ay + 0.5) * s" in divider_branch and "(ax + 0.5) * s" in divider_branch, \
+        "divider stroke is not centered within its LED pixel"
+
+
+def test_gauge_radii_are_clamped_to_zero():
+    """An imported design can carry a small gauge with a wide lineWidth --
+    width=1, height=1, lineWidth=3 sends a negative radius into
+    ctx.ellipse(), which throws IndexSizeError and aborts render() for every
+    element still to be drawn, not just the gauge."""
+    body = _function_source(CANVAS, "_drawElement")
+    gauge_branch = body[body.index("case 'gauge'"):]
+    assert re.search(r"Math\.max\(0,\s*rx\s*-\s*lwPx\s*/\s*2\)", gauge_branch), \
+        "gauge x-radius is not clamped to zero"
+    assert re.search(r"Math\.max\(0,\s*ry\s*-\s*lwPx\s*/\s*2\)", gauge_branch), \
+        "gauge y-radius is not clamped to zero"
+    assert "rx - lwPx / 2" not in re.sub(r"Math\.max\(0,\s*rx\s*-\s*lwPx\s*/\s*2\)", "", gauge_branch), \
+        "an unclamped gauge radius is still passed to ctx.ellipse()"
+
+
+#: ELEMENT_DEFAULTS in composer-canvas.js gives exactly these types a
+#: `binding` object; verified against that file rather than assumed.
+BOUND_TYPES = ["dynamic_text", "progress_bar", "countdown", "pips", "sparkline", "gauge"]
+
+
+def test_bound_types_constant_lists_every_bound_element_type():
+    """ELEMENT_DEFAULTS in composer-canvas.js gives dynamic_text, progress_bar,
+    countdown, pips, sparkline and gauge a `binding` object. If a type is added
+    to (or removed from) that list without updating APP's BOUND_TYPES, the
+    checks below drift out of sync silently -- this pins the two together."""
+    src = APP.read_text()
+    match = re.search(r"const BOUND_TYPES = \[([^\]]*)\];", src)
+    assert match, "BOUND_TYPES constant not found in composer-app.js"
+    declared = {t.strip().strip("'\"") for t in match.group(1).split(",") if t.strip()}
+    assert declared == set(BOUND_TYPES), \
+        f"BOUND_TYPES {declared} does not match the bound element types {set(BOUND_TYPES)}"
+
+
+@pytest.mark.parametrize("method", ["_isBound", "removeConfigVar", "_validateBeforeExport"])
+def test_binding_checks_cover_every_bound_element_type(method):
+    """Only dynamic_text and progress_bar were checked, so a countdown, pips,
+    sparkline or gauge element with an empty binding passed export validation
+    silently, and deleting a config var still used by one of them gave no
+    warning -- the plugin would read a now-missing key at runtime with no
+    indication why."""
+    body = _method_source(APP, method)
+    assert "BOUND_TYPES.includes(e.type)" in body, \
+        f"{method} does not check every bound element type via BOUND_TYPES"
+    assert "e.type === 'dynamic_text'" not in body, \
+        f"{method} still hardcodes only dynamic_text instead of BOUND_TYPES"

test/test_composer_path_containment.py

@@ -0,0 +1,225 @@
+"""A composer plugin id must never resolve outside the plugins directory.
+
+CodeQL reported sixteen high-severity py/path-injection alerts against
+web_interface/blueprints/composer.py: a request-supplied plugin_id reaching
+Path(plugins_dir) / plugin_id, which is then created, written to, deleted
+(shutil.rmtree) and read back.
+
+The id was already validated by an anchored regex, so every traversal payload
+was in fact rejected. What was missing was the guarantee living *with* the path
+building rather than in a regex several hundred lines away -- loosen that regex
+later and the traversal opens silently, with nothing at the filesystem boundary
+to catch it. _plugin_dir() closes that, and is the form static analysis can see.
+"""
+import sys
+from pathlib import Path
+from unittest.mock import MagicMock
+
+import pytest
+
+sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
+
+from web_interface.blueprints import composer as C  # noqa: E402
+
+#: Anything that has ever been used to climb out of a directory.
+TRAVERSAL = [
+    "../../etc/passwd", "..", ".", "a/../../etc", "good/../../..",
+    "/etc/passwd", "//etc/passwd", "a\\..\\..", "a%2f..%2f..",
+    "....//....//etc", "a/./../../etc", "~", "~root",
+    "plugin/../../../../../../etc/shadow",
+]
+
+#: Rejected for shape, not traversal -- but rejected all the same.
+MALFORMED = ["", "A-upper", "1-leading-digit", "-leading-dash", "has_underscore",
+             "has space", "has.dot", "a" * 64, "plugin\n", "plugin\n../../etc",
+             "\n", "plug\x00in"]
+
+
+@pytest.fixture
+def plugins_dir(tmp_path, monkeypatch):
+    base = tmp_path / "plugin-repos"
+    base.mkdir()
+    monkeypatch.setattr(C.composer_bp, "plugins_dir", str(base), raising=False)
+    return base
+
+
+@pytest.mark.parametrize("payload", TRAVERSAL)
+def test_traversal_payloads_are_refused(plugins_dir, payload):
+    assert C._plugin_dir(payload) is None
+
+
+@pytest.mark.parametrize("payload", MALFORMED)
+def test_malformed_ids_are_refused(plugins_dir, payload):
+    assert C._plugin_dir(payload) is None
+
+
+@pytest.mark.parametrize("payload", ["a", "my-plugin", "x9", "a" * 63])
+def test_valid_ids_resolve_inside_the_base(plugins_dir, payload):
+    resolved = C._plugin_dir(payload)
+    assert resolved is not None, f"{payload!r} was rejected but is valid"
+    assert resolved.parent == plugins_dir.resolve(), (
+        f"{payload!r} resolved to {resolved}, outside {plugins_dir}")
+
+
+def test_no_payload_can_escape_even_if_the_regex_is_loosened(plugins_dir, monkeypatch):
+    """The containment check must stand on its own.
+
+    This is the whole point of resolving at the filesystem boundary: if the id
+    pattern is ever relaxed, traversal must still be impossible. Replace the
+    regex with one that permits slashes and dots, then re-run the payloads.
+    """
+    import re
+    monkeypatch.setattr(C, "_PLUGIN_ID_RE", re.compile(r"\A[\w./\\~-]+\Z"))
+    import os
+    escaped = []
+    base = os.path.realpath(str(plugins_dir))
+    for payload in TRAVERSAL:
+        resolved = C._plugin_dir(payload)
+        if resolved is None:
+            continue
+        real = os.path.realpath(str(resolved))
+        if real != base and os.path.commonpath([base, real]) != base:
+            escaped.append((payload, real))
+    assert not escaped, f"these escaped the base with a loosened regex: {escaped}"
+
+
+def test_a_sibling_directory_with_a_shared_prefix_is_not_inside(tmp_path, monkeypatch):
+    """commonpath, not startswith.
+
+    "/x/plugins-evil" starts with "/x/plugins" but is a different directory, so
+    a prefix test would accept it.
+    """
+    base = tmp_path / "plugins"
+    base.mkdir()
+    (tmp_path / "plugins-evil").mkdir()
+    monkeypatch.setattr(C.composer_bp, "plugins_dir", str(base), raising=False)
+    import re
+    # Neutralise the two layers in front so this exercises the containment
+    # check itself; otherwise secure_filename rejects the payload first and a
+    # startswith regression would go unnoticed here.
+    monkeypatch.setattr(C, "_PLUGIN_ID_RE", re.compile(r"\A[\w./\\~-]+\Z"))
+    monkeypatch.setattr(C, "secure_filename", lambda v: v)
+    assert C._plugin_dir("../plugins-evil") is None
+
+
+def test_containment_still_holds_if_the_sanitiser_is_defeated(plugins_dir, monkeypatch):
+    """Each layer is tested on its own, not just the stack.
+
+    secure_filename's equality guard rejects every traversal payload before the
+    containment check sees it, so removing containment does not fail the other
+    tests -- which would make it look load-bearing when it is not. Neutralise
+    the regex *and* the sanitiser, and the realpath/commonpath check must still
+    refuse everything on its own.
+    """
+    import re
+    monkeypatch.setattr(C, "_PLUGIN_ID_RE", re.compile(r"\A[\w./\\~-]+\Z"))
+    monkeypatch.setattr(C, "secure_filename", lambda v: v)
+    import os
+    base = os.path.realpath(str(plugins_dir))
+    escaped = []
+    for payload in TRAVERSAL:
+        resolved = C._plugin_dir(payload)
+        if resolved is None:
+            continue
+        real = os.path.realpath(str(resolved))
+        # Inside the base is fine -- "...." and "~" are ordinary directory
+        # names on Linux, so they are not escapes. What must never happen is
+        # landing outside the base, or on the base itself: install() rmtrees
+        # its target, so the plugins root resolving to a "plugin" would wipe
+        # every installed plugin.
+        if real == base or os.path.commonpath([base, real]) != base:
+            escaped.append((payload, real))
+    assert not escaped, f"containment alone let these through: {escaped}"
+
+
+def test_secure_filename_never_rewrites_an_accepted_id(plugins_dir):
+    """The sanitiser must be a no-op on everything the regex accepts.
+
+    If secure_filename ever altered an accepted id, _plugin_dir would resolve
+    to a *different* plugin's directory than the caller asked for -- a silent
+    redirect, which is worse than a refusal. The guard turns that into a
+    refusal; this proves the guard never has to fire in practice.
+    """
+    import random
+    from werkzeug.utils import secure_filename
+    random.seed(1)
+    alphabet = "abcdefghijklmnopqrstuvwxyz0123456789-"
+    altered = []
+    for _ in range(2000):
+        n = random.randint(1, 63)
+        cand = random.choice("abcdefghijklmnopqrstuvwxyz") + "".join(
+            random.choice(alphabet) for _ in range(n - 1))
+        if C._PLUGIN_ID_RE.match(cand) and secure_filename(cand) != cand:
+            altered.append((cand, secure_filename(cand)))
+    assert not altered, f"secure_filename rewrote accepted ids: {altered[:5]}"
+
+
+def test_a_trailing_newline_is_not_a_valid_id():
+    r"""Python's `$` also matches before a trailing newline, so the original
+    `^...$` accepted "myplugin\n" and would have created a directory whose
+    name ends in one. \Z does not."""
+    assert C._PLUGIN_ID_RE.match("myplugin") is not None
+    assert C._PLUGIN_ID_RE.match("myplugin\n") is None
+
+
+# --- font serving -----------------------------------------------------------
+
+FONT_TRAVERSAL = [
+    "../../../etc/passwd", "../config/config.json", "..%2f..%2fetc%2fpasswd",
+    "PressStart2P-Regular.ttf/../../../etc/passwd", "/etc/passwd", "",
+    "PressStart2P-Regular.TTF",          # case differs -> not the allowlisted name
+    "PressStart2P-Regular.ttf ",         # trailing space
+]
+
+
+def test_serve_font_refuses_a_file_that_exists_but_is_not_allowlisted(monkeypatch, tmp_path):
+    """The allowlist must be what refuses it, not a missing file.
+
+    Asserting 404 on traversal payloads proves nothing here: Flask's router
+    will not match a path segment containing '/', and everything else 404s
+    simply because no such file exists. Put a real, readable file next to the
+    fonts and confirm it is still refused -- that is the allowlist working.
+    """
+    fonts = tmp_path / "assets" / "fonts"
+    fonts.mkdir(parents=True)
+    (fonts / "id_rsa.ttf").write_bytes(b"PRIVATE KEY")
+    monkeypatch.setattr(C.composer_bp, "project_root", str(tmp_path), raising=False)
+    app = __import__("flask").Flask(__name__)
+    app.register_blueprint(C.composer_bp)
+    with app.test_client() as client:
+        resp = client.get("/api/fonts/id_rsa.ttf")
+    assert resp.status_code == 404, (
+        "a readable non-allowlisted file was served; the allowlist is not gating")
+    assert b"PRIVATE KEY" not in resp.data
+
+
+@pytest.mark.parametrize("payload", FONT_TRAVERSAL)
+def test_serve_font_refuses_anything_not_allowlisted(payload, monkeypatch, tmp_path):
+    """The name reaching the filesystem must come from the allowlist constant.
+
+    _ALLOWED_FONTS gates this endpoint, so nothing here was ever exploitable.
+    Building the path from the matched constant rather than the request value
+    is what makes that provable -- and it is why CodeQL reported two
+    high-severity py/path-injection alerts on an endpoint that was already
+    safe.
+    """
+    monkeypatch.setattr(C.composer_bp, "project_root", str(tmp_path), raising=False)
+    app = C.composer_bp.name and __import__("flask").Flask(__name__)
+    app.register_blueprint(C.composer_bp)
+    with app.test_client() as client:
+        resp = client.get(f"/api/fonts/{payload}")
+    assert resp.status_code in (404, 405, 308), (
+        f"{payload!r} was not refused (status {resp.status_code})")
+
+
+def test_serve_font_still_serves_each_allowlisted_font(monkeypatch, tmp_path):
+    fonts = tmp_path / "assets" / "fonts"
+    fonts.mkdir(parents=True)
+    monkeypatch.setattr(C.composer_bp, "project_root", str(tmp_path), raising=False)
+    app = __import__("flask").Flask(__name__)
+    app.register_blueprint(C.composer_bp)
+    for name in C._ALLOWED_FONTS:
+        (fonts / name).write_bytes(b"\x00\x01ttf")
+        with app.test_client() as client:
+            resp = client.get(f"/api/fonts/{name}")
+        assert resp.status_code == 200, f"{name} should be served, got {resp.status_code}"

test/test_discovery_path_contract.py

@@ -116,77 +116,6 @@ class TestFallbackDivergence:
             plugin_dir / "config_schema.json"
 
 
-class TestRegistryIdVersusDirectoryName:
-    """install_plugin() renames a plugin's directory to the MANIFEST id when it
-    differs from the REGISTRY id, so `stocks` in the registry lands in
-    `ledmatrix-stocks/`. Every path lookup is by directory name, so
-    update_plugin("stocks") used to find nothing and report the plugin as not
-    installed -- silently. Four installed plugins hit this in practice
-    (leaderboard, music, stocks, weather): the user clicks update, nothing
-    happens, no error, and they stay on a stale version indefinitely.
-    """
-
-    def test_resolves_a_plugin_whose_directory_name_differs_from_its_id(self, tmp_path):
-        configured = tmp_path / "plugin-repos"
-        configured.mkdir()
-        plugin_dir = _write_plugin(configured, "stocks", dir_name="ledmatrix-stocks")
-
-        store = PluginStoreManager(plugins_dir=str(configured))
-        assert store._find_plugin_path("stocks") == plugin_dir
-
-    def test_direct_directory_hit_still_wins(self, tmp_path):
-        """The manifest scan is a last resort. A directory named for the id
-        must still be preferred, so the two documented lookups above keep
-        their exact meaning."""
-        configured = tmp_path / "plugin-repos"
-        configured.mkdir()
-        direct = _write_plugin(configured, "demo", dir_name="demo")
-        # A second directory whose manifest claims the same id.
-        _write_plugin(configured, "demo", dir_name="zz-other-demo")
-
-        store = PluginStoreManager(plugins_dir=str(configured))
-        assert store._find_plugin_path("demo") == direct
-
-    def test_unknown_id_is_still_not_found(self, tmp_path):
-        configured = tmp_path / "plugin-repos"
-        configured.mkdir()
-        _write_plugin(configured, "stocks", dir_name="ledmatrix-stocks")
-
-        store = PluginStoreManager(plugins_dir=str(configured))
-        assert store._find_plugin_path("no-such-plugin") is None
-
-    def test_half_finished_installs_are_not_resurrected(self, tmp_path):
-        """A directory renamed aside during install/rollback carries a valid
-        manifest. Matching one would report a ghost plugin as installed."""
-        configured = tmp_path / "plugin-repos"
-        configured.mkdir()
-        _write_plugin(configured, "ghost",
-                      dir_name="ghost.standalone-backup-1234")
-
-        store = PluginStoreManager(plugins_dir=str(configured))
-        assert store._find_plugin_path("ghost") is None
-
-    def test_unreadable_manifest_does_not_break_the_scan(self, tmp_path):
-        configured = tmp_path / "plugin-repos"
-        configured.mkdir()
-        broken = configured / "broken-plugin"
-        broken.mkdir()
-        (broken / "manifest.json").write_text("{ not json")
-        wanted = _write_plugin(configured, "stocks", dir_name="ledmatrix-stocks")
-
-        store = PluginStoreManager(plugins_dir=str(configured))
-        assert store._find_plugin_path("stocks") == wanted
-
-    def test_sibling_plugins_dir_is_searched_too(self, tmp_path):
-        configured = tmp_path / "plugin-repos"
-        configured.mkdir()
-        legacy = _write_plugin(tmp_path / "plugins", "weather",
-                               dir_name="ledmatrix-weather")
-
-        store = PluginStoreManager(plugins_dir=str(configured))
-        assert store._find_plugin_path("weather") == legacy
-
-
 class TestStandaloneBackupContract:
     def test_discovery_skips_backup_dirs(self, tmp_path):
         plugins_dir = tmp_path / "plugins"

test/test_display_dirty_tracking.py

@@ -47,17 +47,12 @@ class _SwapSpy:
     def __init__(self, matrix):
         self.matrix = matrix
         self.count = 0
-        self.last_frame_hold = None
         self._orig = matrix.SwapOnVSync
 
     def __enter__(self):
-        def counting(canvas, *args):
-            # *args carries framerate_fraction, which display_manager passes so
-            # a frame can be held for several refreshes. Signature must match
-            # the real binding or the spy hides a TypeError as a failed push.
+        def counting(canvas):
             self.count += 1
-            self.last_frame_hold = args[0] if args else 1
-            return self._orig(canvas, *args)
+            return self._orig(canvas)
         self.matrix.SwapOnVSync = counting
         return self
 
@@ -114,11 +109,6 @@ class TestDirtyTracking:
         dm.draw.rectangle([0, 0, 30, 8], fill=(255, 255, 0))
         dm.update_display()   # push + snapshot write (first frame)
         assert os.path.exists(dm._snapshot_path)
-        # Backdate the file so the "was it bumped?" check below cannot be
-        # defeated by filesystem mtime granularity -- on Windows two writes in
-        # the same tick get identical timestamps, which made this test fail
-        # roughly two runs in three regardless of the code under test.
-        os.utime(dm._snapshot_path, (time.time() - 60, time.time() - 60))
         first_mtime = os.path.getmtime(dm._snapshot_path)
 
         # Age the write/touch bookkeeping past TOUCH_INTERVAL so the next
@@ -135,62 +125,6 @@ class TestDirtyTracking:
         assert os.path.getmtime(dm._snapshot_path) > first_mtime
 
 
-class TestScrollLock:
-    """Dirty tracking must not skip the panel push while a scroll is running.
-
-    SwapOnVSync is what paces the render loop, so skipping it also skips the
-    wait for the panel. A duplicate frame therefore returns early -- ~8ms
-    instead of ~10ms on a 100Hz panel -- which advances the strip only 0.8px
-    instead of 1.0px, which makes the NEXT frame more likely to be a duplicate
-    too. That is self-sustaining: measured at ~20% duplicate frames mid-scroll
-    on the odds ticker against essentially zero on a lighter plugin with
-    identical scroll settings. Pushing an identical frame costs one canvas
-    copy; falling out of vsync lock costs smooth motion.
-    """
-
-    def test_identical_frames_still_push_while_scrolling(self, dm):
-        dm.draw.rectangle([0, 0, 12, 12], fill=(0, 0, 255))
-        dm.update_display()
-        dm.set_scrolling_state(True)
-        try:
-            with _SwapSpy(dm.matrix) as spy:
-                dm.update_display()
-                dm.update_display()
-                dm.update_display()
-            assert spy.count == 3, "scrolling must stay locked to the panel"
-        finally:
-            dm.set_scrolling_state(False)
-
-    def test_identical_frames_are_skipped_when_not_scrolling(self, dm):
-        """The optimisation still applies to static content."""
-        dm.set_scrolling_state(False)
-        dm.draw.rectangle([0, 0, 14, 14], fill=(255, 0, 255))
-        dm.update_display()
-        with _SwapSpy(dm.matrix) as spy:
-            dm.update_display()
-            dm.update_display()
-        assert spy.count == 0
-
-    def test_stale_scrolling_state_stops_forcing_pushes(self, dm):
-        """A plugin that stops scrolling without saying so must not pin the
-        panel into always-push forever. is_currently_scrolling() expires on
-        its own inactivity threshold, and the skip has to come back with it."""
-        dm.draw.rectangle([0, 0, 16, 16], fill=(0, 255, 255))
-        dm.update_display()
-        dm.set_scrolling_state(True)
-        try:
-            # Backdate the activity marker past the inactivity threshold.
-            dm._scrolling_state['last_scroll_activity'] = (
-                time.time() - dm._scrolling_state['scroll_inactivity_threshold'] - 1.0)
-            assert dm.is_currently_scrolling() is False
-            with _SwapSpy(dm.matrix) as spy:
-                dm.update_display()
-                dm.update_display()
-            assert spy.count == 0
-        finally:
-            dm.set_scrolling_state(False)
-
-
 class TestKillSwitch:
     def test_dirty_tracking_can_be_disabled(self, dm):
         dm._dirty_tracking_enabled = False
@@ -226,89 +160,3 @@ class TestKillSwitch:
 
 if __name__ == "__main__":
     sys.exit(pytest.main([__file__, "-v"]))
-
-
-class TestFrameHold:
-    """Holding a frame for N refreshes is how a scroll runs slower than one
-    pixel per refresh without fractional pixel positions."""
-
-    def test_hold_reaches_swap_on_vsync(self, dm):
-        dm.set_scrolling_state(True)
-        dm.set_frame_hold(3)
-        try:
-            dm.draw.rectangle([0, 0, 9, 9], fill=(120, 0, 200))
-            with _SwapSpy(dm.matrix) as spy:
-                dm.update_display()
-            assert spy.count == 1
-            assert spy.last_frame_hold == 3
-        finally:
-            dm.set_scrolling_state(False)
-
-    def test_default_is_every_refresh(self, dm):
-        dm.set_scrolling_state(True)
-        try:
-            dm.draw.rectangle([0, 0, 11, 11], fill=(0, 120, 200))
-            with _SwapSpy(dm.matrix) as spy:
-                dm.update_display()
-            assert spy.last_frame_hold == 1
-        finally:
-            dm.set_scrolling_state(False)
-
-    def test_hold_resets_when_scrolling_stops(self, dm):
-        """One plugin's pacing must not leak into whatever is on screen next."""
-        dm.set_scrolling_state(True)
-        dm.set_frame_hold(5)
-        dm.set_scrolling_state(False)
-        dm.set_scrolling_state(True)
-        try:
-            dm.draw.rectangle([0, 0, 13, 13], fill=(200, 120, 0))
-            with _SwapSpy(dm.matrix) as spy:
-                dm.update_display()
-            assert spy.last_frame_hold == 1
-        finally:
-            dm.set_scrolling_state(False)
-
-    @pytest.mark.parametrize("bad,expected", [(0, 1), (-4, 1), (None, 1), ("x", 1)])
-    def test_unusable_holds_are_ignored_or_floored(self, dm, bad, expected):
-        dm.set_frame_hold(bad)
-        assert dm._frame_hold == expected
-
-
-class TestFrameHoldLifetime:
-    """The hold must last exactly as long as the scroll that asked for it.
-
-    Plugins share one display manager. A hold applied at plugin construction is
-    wiped the moment any *other* plugin finishes scrolling, so by the time the
-    first plugin renders it is back to one pixel per refresh -- the speed reads
-    correct in the log and is wrong on the panel.
-    """
-
-    def test_scrolling_state_carries_the_hold(self, dm):
-        dm.set_scrolling_state(True, frame_hold=4)
-        try:
-            dm.draw.rectangle([0, 0, 7, 7], fill=(10, 200, 10))
-            with _SwapSpy(dm.matrix) as spy:
-                dm.update_display()
-            assert spy.last_frame_hold == 4
-        finally:
-            dm.set_scrolling_state(False)
-
-    def test_another_plugin_stopping_does_not_strand_a_hold(self, dm):
-        dm.set_scrolling_state(True, frame_hold=3)
-        dm.set_scrolling_state(False)          # some other plugin finishes
-        dm.set_scrolling_state(True)           # a plugin that wants no hold
-        try:
-            dm.draw.rectangle([0, 0, 6, 6], fill=(200, 10, 10))
-            with _SwapSpy(dm.matrix) as spy:
-                dm.update_display()
-            assert spy.last_frame_hold == 1
-        finally:
-            dm.set_scrolling_state(False)
-
-    def test_default_keeps_previous_behaviour(self, dm):
-        """Callers that never heard of frame holds get one frame per refresh."""
-        dm.set_scrolling_state(True)
-        try:
-            assert dm._frame_hold == 1
-        finally:
-            dm.set_scrolling_state(False)

test/test_display_double_parity.py

@@ -1,69 +0,0 @@
-"""The visual-test double must not drift from the real DisplayManager.
-
-Both directions of drift are silent and both are damaging:
-
-  * the double accepts an argument production does not -- the call passes
-    every harness run and raises TypeError on the panel, which is precisely
-    the failure a safety harness exists to prevent;
-  * the double lacks an argument production has -- every plugin that
-    legitimately uses it fails every render, and the harness blames the
-    plugin.
-
-`set_scrolling_state` has been each of those in turn across two branches, so
-the parity is asserted rather than remembered.
-
-Read with ast rather than imported: src/display_manager.py imports rgbmatrix
-at module scope, which is absent anywhere without the panel library, and this
-check should hold on a laptop and in CI as well as on a Pi.
-"""
-
-import ast
-from pathlib import Path
-
-import pytest
-
-ROOT = Path(__file__).resolve().parent.parent
-REAL = ROOT / "src" / "display_manager.py"
-DOUBLE = ROOT / "src" / "plugin_system" / "testing" / "visual_display_manager.py"
-
-#: Methods a plugin calls on whichever manager it is handed.
-SHARED_METHODS = ["set_scrolling_state", "is_currently_scrolling"]
-
-
-def _signature(path: Path, class_hint: str, method: str):
-    """(name, default-repr) pairs for `method`, or None if it is absent."""
-    tree = ast.parse(path.read_text(encoding="utf-8"))
-    for node in ast.walk(tree):
-        if not isinstance(node, ast.ClassDef) or class_hint not in node.name:
-            continue
-        for item in node.body:
-            if isinstance(item, (ast.FunctionDef, ast.AsyncFunctionDef)) and item.name == method:
-                args = [a.arg for a in item.args.args if a.arg != "self"]
-                pad = [None] * (len(args) - len(item.args.defaults))
-                defaults = pad + [ast.unparse(d) for d in item.args.defaults]
-                return list(zip(args, defaults))
-    return None
-
-
-@pytest.mark.parametrize("method", SHARED_METHODS)
-def test_the_double_matches_production(method):
-    real = _signature(REAL, "DisplayManager", method)
-    double = _signature(DOUBLE, "DisplayManager", method)
-
-    assert real is not None, f"DisplayManager lost {method}"
-    assert double is not None, \
-        f"the test double is missing {method}, so every plugin using it fails to render"
-    assert double == real, (
-        f"{method} has drifted: production takes {real}, the double takes {double}. "
-        "A double that is more permissive hides a production TypeError; one that "
-        "is less permissive fails plugins that are actually correct."
-    )
-
-
-def test_frame_hold_is_accepted_by_both():
-    """The specific argument that has drifted twice."""
-    for path, label in ((REAL, "DisplayManager"), (DOUBLE, "VisualTestDisplayManager")):
-        params = dict(_signature(path, "DisplayManager", "set_scrolling_state") or [])
-        assert "frame_hold" in params, f"{label} does not accept frame_hold"
-        assert params["frame_hold"] == "1", \
-            f"{label} must default frame_hold to 1 so existing callers are unaffected"

test/test_harness_empty_claimed.py

@@ -101,61 +101,3 @@ class TestSkippedResults:
         r = _result(None, returned=None)
         check_empty_claimed([r], strict=True)
         assert r.empty_claimed is None
-
-
-class TestSettleRecordsLaterFailures:
-    """A mode that renders one good frame and then crashes is broken.
-
-    _settle_loop re-renders a mode that came back blank, to give a scroll or an
-    animation time to put something on the panel. Swallowing an exception from
-    those later frames meant the harness reported a passing result for a mode
-    that crashes as soon as it is asked for a second frame -- exactly the kind
-    of defect the harness exists to catch.
-    """
-
-    class Boom:
-        """Renders once, then raises."""
-
-        def __init__(self):
-            self.calls = 0
-
-        def display(self, force_clear=False):
-            self.calls += 1
-            raise RuntimeError("second frame exploded")
-
-    def _settle(self, inst, dm, result):
-        from src.plugin_system.testing import harness
-        harness._settle_loop(inst, "mode", dm, result, None)
-
-    def test_the_exception_is_recorded_on_the_result(self, monkeypatch):
-        from src.plugin_system.testing import harness
-        # Keep the probe short; this test is about the error, not the pacing.
-        monkeypatch.setattr(harness, "EMPTY_RECHECK_FRAMES", 1)
-        monkeypatch.setattr(harness, "EMPTY_RECHECK_STEP", 0)
-
-        result = _result(_blank())
-        assert result.error is None
-        self._settle(self.Boom(), _FakeDM(), result)
-
-        assert result.error is not None, "a crash on a later frame was swallowed"
-        assert "second frame exploded" in result.error
-
-    def test_the_already_captured_frame_is_kept(self, monkeypatch):
-        from src.plugin_system.testing import harness
-        monkeypatch.setattr(harness, "EMPTY_RECHECK_FRAMES", 1)
-        monkeypatch.setattr(harness, "EMPTY_RECHECK_STEP", 0)
-
-        image = _blank()
-        result = _result(image)
-        self._settle(self.Boom(), _FakeDM(), result)
-        assert result.image is image, "the good frame was discarded along with the error"
-
-
-class _FakeDM:
-    """Minimal display-manager double for _settle_loop."""
-
-    def get_image(self):
-        return _blank()
-
-    def check_overflow(self):
-        return None

test/test_on_demand_mailbox.py

@@ -1,106 +0,0 @@
-"""The on-demand request mailbox: how often it is read, and how it is consumed.
-
-The mailbox is a cache key the web process writes and the display process
-reads. Two properties matter and neither is obvious from the call site:
-
-  * it is polled after every rendered frame, so an uncached read here is a
-    disk read at frame rate;
-  * consuming it must not throw away a request that arrived while the previous
-    one was being processed.
-"""
-
-from unittest.mock import MagicMock
-
-import pytest
-
-
-class TestPollingIsBounded:
-    """_poll_on_demand_requests runs ~125x/second on a scrolling mode.
-
-    The read is deliberately uncached (memory_ttl=0) because a cached one
-    pinned the first request for an hour. That makes the call a real disk read,
-    so it needs a floor -- without one it was ~125 reads per second to find
-    nothing at all.
-    """
-
-    def test_first_call_always_reads(self, test_display_controller):
-        c = test_display_controller
-        c.cache_manager.get = MagicMock(return_value=None)
-        c._poll_on_demand_requests()
-        assert c.cache_manager.get.call_count == 1
-
-    def test_immediate_second_call_does_not_read(self, test_display_controller):
-        c = test_display_controller
-        c.cache_manager.get = MagicMock(return_value=None)
-        c._poll_on_demand_requests()
-        for _ in range(50):
-            c._poll_on_demand_requests()
-        assert c.cache_manager.get.call_count == 1, "polling was not bounded"
-
-    def test_reads_again_once_the_interval_has_passed(self, test_display_controller, monkeypatch):
-        c = test_display_controller
-        c.cache_manager.get = MagicMock(return_value=None)
-        clock = {"t": 1000.0}
-        monkeypatch.setattr("src.display_controller.time.monotonic", lambda: clock["t"])
-
-        c._poll_on_demand_requests()
-        clock["t"] += c.ON_DEMAND_POLL_INTERVAL / 2
-        c._poll_on_demand_requests()
-        assert c.cache_manager.get.call_count == 1, "read before the interval elapsed"
-
-        clock["t"] += c.ON_DEMAND_POLL_INTERVAL
-        c._poll_on_demand_requests()
-        assert c.cache_manager.get.call_count == 2
-
-    def test_the_interval_is_short_enough_to_feel_instant(self, test_display_controller):
-        # A person clicking in the web UI must not notice the floor.
-        assert test_display_controller.ON_DEMAND_POLL_INTERVAL <= 0.5
-
-
-class TestMailboxIsConsumedByIdentity:
-    """Deleting whatever is in the mailbox loses a request that raced in."""
-
-    def _arrange(self, controller, first, later):
-        """Mailbox returns `first`, then `later` on the pre-delete re-read."""
-        controller.on_demand_active = False
-        controller.on_demand_request_id = None
-        controller._last_on_demand_poll = None
-        reads = iter([first, later])
-
-        def fake_get(key, *a, **kw):
-            if key == 'display_on_demand_request':
-                return next(reads, later)
-            return None  # processed-id lookup
-
-        controller.cache_manager.get = MagicMock(side_effect=fake_get)
-        controller.cache_manager.set = MagicMock()
-        controller.cache_manager.delete = MagicMock()
-        controller._activate_on_demand = MagicMock()
-
-    REQ_A = {'request_id': 'A', 'action': 'start', 'plugin_id': 'p', 'mode': 'm'}
-    REQ_B = {'request_id': 'B', 'action': 'start', 'plugin_id': 'p', 'mode': 'm'}
-
-    def test_own_request_is_deleted(self, test_display_controller):
-        c = test_display_controller
-        self._arrange(c, self.REQ_A, self.REQ_A)
-        c._poll_on_demand_requests()
-        c.cache_manager.delete.assert_called_once_with('display_on_demand_request')
-
-    def test_a_newer_request_is_left_for_the_next_poll(self, test_display_controller):
-        c = test_display_controller
-        self._arrange(c, self.REQ_A, self.REQ_B)
-        c._poll_on_demand_requests()
-        assert c.cache_manager.delete.call_count == 0, \
-            "request B was deleted without ever being processed"
-
-    def test_an_already_empty_mailbox_is_still_cleared(self, test_display_controller):
-        c = test_display_controller
-        self._arrange(c, self.REQ_A, None)
-        c._poll_on_demand_requests()
-        c.cache_manager.delete.assert_called_once_with('display_on_demand_request')
-
-    def test_the_request_is_still_processed(self, test_display_controller):
-        c = test_display_controller
-        self._arrange(c, self.REQ_A, self.REQ_B)
-        c._poll_on_demand_requests()
-        c._activate_on_demand.assert_called_once()

test/test_scroll_config.py

@@ -1,467 +0,0 @@
-"""Tests for the shared scroll configuration resolver."""
-
-import logging
-import sys
-from pathlib import Path
-
-import pytest
-
-sys.path.insert(0, str(Path(__file__).resolve().parent.parent))
-
-from src.common.scroll_config import (  # noqa: E402
-    DEFAULT_PIXELS_PER_SECOND,
-    MAX_PIXELS_PER_FRAME,
-    crisp_ladder,
-    solve_crisp,
-    MAX_PIXELS_PER_SECOND,
-    MIN_PIXELS_PER_SECOND,
-    ScrollSettings,
-    configure,
-    refresh_hz_from_config,
-    resolve,
-)
-
-
-class FakeHelper:
-    """Records what configure() applied."""
-
-    def __init__(self, with_optional=True):
-        self.speed = None
-        self.frame_based = None
-        self.target_fps = None
-        if not with_optional:
-            del FakeHelper.set_frame_based_scrolling
-            del FakeHelper.set_target_fps
-
-    def set_scroll_speed(self, speed):
-        self.speed = speed
-
-    def set_frame_based_scrolling(self, enabled):
-        self.frame_based = enabled
-
-    def set_target_fps(self, fps):
-        self.target_fps = fps
-
-
-class FakeDisplayManager:
-    """Records the frame hold configure() applies."""
-
-    def __init__(self):
-        self.hold = None
-
-    def set_frame_hold(self, refreshes):
-        self.hold = refreshes
-
-
-class MinimalHelper:
-    """An older helper exposing only set_scroll_speed."""
-
-    def __init__(self):
-        self.speed = None
-
-    def set_scroll_speed(self, speed):
-        self.speed = speed
-
-
-class TestPrecedence:
-    def test_display_options_pair_wins(self):
-        s = resolve({"display_options": {"scroll_speed": 1.0, "scroll_delay": 0.01}})
-        assert s.pixels_per_second == 100.0
-        assert s.source == "display_options.scroll_speed/delay"
-
-    def test_display_block_used_when_options_absent(self):
-        s = resolve({"display": {"scroll_speed": 2.0, "scroll_delay": 0.01}})
-        assert s.pixels_per_second == 200.0
-        assert s.source == "display.scroll_speed/delay"
-
-    def test_root_pair_used_when_both_blocks_absent(self):
-        s = resolve({"scroll_speed": 1.0, "scroll_delay": 0.02})
-        assert s.pixels_per_second == 50.0
-
-    def test_pixels_per_second_used_when_no_pair_given(self):
-        s = resolve({"display_options": {"scroll_pixels_per_second": 120.0}})
-        assert s.pixels_per_second == 120.0
-        assert s.source == "display_options.scroll_pixels_per_second"
-
-    def test_global_display_is_the_last_resort_before_default(self):
-        s = resolve({}, {"display": {"scroll_speed": 1.0, "scroll_delay": 0.005}})
-        assert s.pixels_per_second == 200.0
-
-    def test_default_when_nothing_configured(self):
-        s = resolve({}, {})
-        assert s.pixels_per_second == DEFAULT_PIXELS_PER_SECOND
-        assert s.source == "default"
-
-
-class TestDeprecatedKeyCannotOverrideExplicitPair:
-    """Regression for ChuckBuilds/ledmatrix-plugins#408.
-
-    odds-ticker ranked scroll_pixels_per_second above the documented
-    scroll_speed/scroll_delay pair. Because that key carries a schema default,
-    the documented settings became unreachable for every user and the ticker
-    silently ran at the default speed. The pair must win.
-    """
-
-    def test_pair_beats_pixels_per_second_in_the_same_block(self):
-        s = resolve(
-            {
-                "display_options": {
-                    "scroll_speed": 1.0,
-                    "scroll_delay": 0.01,
-                    "scroll_pixels_per_second": 50.0,  # schema default
-                }
-            }
-        )
-        assert s.pixels_per_second == 100.0
-        assert "scroll_speed/delay" in s.source
-
-    def test_pair_beats_pixels_per_second_in_an_outer_block(self):
-        s = resolve(
-            {
-                "display_options": {"scroll_speed": 1.0, "scroll_delay": 0.01},
-                "scroll_pixels_per_second": 50.0,
-            }
-        )
-        assert s.pixels_per_second == 100.0
-
-
-class TestMalformedValues:
-    @pytest.mark.parametrize("bad", [None, "fast", "", {}, [], float("nan")])
-    def test_unusable_speed_falls_through(self, bad):
-        s = resolve({"display_options": {"scroll_speed": bad, "scroll_delay": 0.01}})
-        assert s.pixels_per_second == DEFAULT_PIXELS_PER_SECOND
-
-    @pytest.mark.parametrize("bad", [0, -5, 0.0])
-    def test_non_positive_values_fall_through(self, bad):
-        s = resolve({"display_options": {"scroll_pixels_per_second": bad}})
-        assert s.pixels_per_second == DEFAULT_PIXELS_PER_SECOND
-
-    def test_booleans_are_not_treated_as_numbers(self):
-        s = resolve({"display_options": {"scroll_pixels_per_second": True}})
-        assert s.pixels_per_second == DEFAULT_PIXELS_PER_SECOND
-
-    def test_zero_delay_does_not_divide_by_zero(self):
-        s = resolve({"display_options": {"scroll_speed": 1.0, "scroll_delay": 0}})
-        assert s.pixels_per_second == DEFAULT_PIXELS_PER_SECOND
-
-    def test_non_dict_blocks_are_ignored(self):
-        s = resolve({"display_options": "nonsense", "display": 5})
-        assert s.pixels_per_second == DEFAULT_PIXELS_PER_SECOND
-
-
-class TestClamping:
-    def test_absurdly_fast_is_clamped(self):
-        s = resolve({"display_options": {"scroll_pixels_per_second": 100000.0}})
-        assert s.pixels_per_second == MAX_PIXELS_PER_SECOND
-        assert s.warning and "clamped" in s.warning
-
-    def test_absurdly_slow_is_clamped(self):
-        s = resolve({"display_options": {"scroll_pixels_per_second": 0.01}})
-        assert s.pixels_per_second == MIN_PIXELS_PER_SECOND
-
-
-class TestWholePixelWarning:
-    """The display rule: whole pixels per refresh, or it judders."""
-
-    def test_no_warning_when_speed_divides_evenly(self):
-        s = resolve({"display_options": {"scroll_pixels_per_second": 100.0}}, refresh_hz=100)
-        assert s.warning is None
-        assert s.pixels_per_frame == pytest.approx(1.0)
-
-    def test_no_warning_at_an_integer_multiple(self):
-        s = resolve({"display_options": {"scroll_pixels_per_second": 200.0}}, refresh_hz=100)
-        assert s.warning is None
-
-    def test_warns_on_a_half_pixel_per_frame(self):
-        s = resolve({"display_options": {"scroll_pixels_per_second": 50.0}}, refresh_hz=100)
-        assert s.warning is not None
-        assert "judder" in s.warning
-        assert "100 px/s" in s.warning
-
-    def test_warns_on_a_fractional_multiple(self):
-        s = resolve({"display_options": {"scroll_pixels_per_second": 130.0}}, refresh_hz=100)
-        assert s.warning is not None
-
-    def test_respects_a_non_default_refresh_rate(self):
-        s = resolve({"display_options": {"scroll_pixels_per_second": 150.0}}, refresh_hz=150)
-        assert s.warning is None
-        assert s.pixels_per_frame == pytest.approx(1.0)
-
-
-class TestConfigure:
-    def test_applies_time_based_mode_and_speed(self):
-        helper = FakeHelper()
-        s = configure(helper, {"display_options": {"scroll_speed": 1.0, "scroll_delay": 0.01}})
-        assert helper.speed == 100.0
-        assert helper.frame_based is False, "must not use the wall-clock step gate"
-        assert helper.target_fps == 100.0
-        assert s.pixels_per_second == 100.0
-
-    def test_works_against_a_helper_without_optional_methods(self):
-        helper = MinimalHelper()
-        configure(helper, {"display_options": {"scroll_pixels_per_second": 100.0}})
-        assert helper.speed == 100.0
-
-    def test_snapping_removes_the_judder_warning(self, caplog):
-        """50px/s cannot be shown in whole pixels at 100Hz *per refresh*, but it
-        can as 1px every 2nd refresh -- so once snapped there is nothing to warn
-        about. resolve() alone still warns; configure() resolves it."""
-        assert "judder" in resolve(
-            {"display_options": {"scroll_pixels_per_second": 50.0}},
-            refresh_hz=100).warning
-        helper, dm = FakeHelper(), FakeDisplayManager()
-        with caplog.at_level(logging.WARNING):
-            settings = configure(
-                helper, {"display_options": {"scroll_pixels_per_second": 50.0}},
-                display_manager=dm)
-        assert settings.warning is None
-        assert not [r for r in caplog.records if "judder" in r.getMessage()]
-
-    def test_reports_the_hold_without_applying_it(self):
-        """The hold belongs to a scroll, not a plugin's lifetime -- one left set
-        at construction is wiped as soon as any other plugin stops scrolling.
-        configure() reports it; the caller passes it to set_scrolling_state."""
-        dm = FakeDisplayManager()
-        s = configure(FakeHelper(), {"display_options": {"scroll_pixels_per_second": 25.0}},
-                      display_manager=dm)
-        assert s.frame_hold == 4, "25px/s at 100Hz is 1px every 4th refresh"
-        assert dm.hold is None, "must not apply the hold behind the caller's back"
-
-    def test_full_speed_needs_no_hold(self):
-        s = configure(FakeHelper(), {"display_options": {"scroll_pixels_per_second": 100.0}},
-                      display_manager=FakeDisplayManager())
-        assert s.frame_hold == 1
-
-    def test_frame_hold_is_one_when_snapping_is_off(self):
-        s = configure(FakeHelper(), {"display_options": {"scroll_pixels_per_second": 50.0}},
-                      snap_to_crisp=False)
-        assert s.frame_hold == 1
-
-    def test_snaps_to_the_nearest_crisp_speed_and_reports_both(self):
-        s = configure(FakeHelper(), {"display_options": {"scroll_pixels_per_second": 45.0}},
-                      display_manager=FakeDisplayManager())
-        assert s.requested_pixels_per_second == 45.0
-        assert s.pixels_per_second == 50.0
-        assert s.crisp is not None and s.crisp.pixels_per_frame == 1
-
-    def test_snapping_can_be_turned_off(self):
-        s = configure(FakeHelper(), {"display_options": {"scroll_pixels_per_second": 45.0}},
-                      snap_to_crisp=False)
-        assert s.pixels_per_second == 45.0
-        assert s.crisp is None
-
-    def test_helper_target_fps_matches_the_presentation_rate(self):
-        """At a hold of 4 the panel still refreshes at 100Hz, but frames are
-        presented at 25/s -- that is the rate the helper should pace to."""
-        helper = FakeHelper()
-        configure(helper, {"display_options": {"scroll_pixels_per_second": 25.0}},
-                  display_manager=FakeDisplayManager())
-        assert helper.target_fps == pytest.approx(25.0)
-
-    def test_returns_settings_describing_the_source(self):
-        s = configure(FakeHelper(), {"display_options": {"scroll_speed": 2.0, "scroll_delay": 0.01}})
-        assert isinstance(s, ScrollSettings)
-        assert "200.0 px/s" in s.describe()
-
-
-class TestRefreshFromConfig:
-    def test_reads_the_hardware_limit(self):
-        assert refresh_hz_from_config(
-            {"display": {"hardware": {"limit_refresh_rate_hz": 150}}}
-        ) == 150.0
-
-    @pytest.mark.parametrize("cfg", [None, {}, {"display": {}}, {"display": {"hardware": {}}},
-                                     "nonsense", {"display": {"hardware": {"limit_refresh_rate_hz": None}}}])
-    def test_falls_back_to_the_default(self, cfg):
-        assert refresh_hz_from_config(cfg) == 100.0
-
-    @pytest.mark.parametrize("cfg", [
-        {"display": "nonsense"},
-        {"display": ["nonsense"]},
-        {"display": 60},
-        {"display": {"hardware": "nonsense"}},
-        {"display": {"hardware": ["nonsense"]}},
-        {"display": {"hardware": 60}},
-    ])
-    def test_malformed_nesting_falls_back_rather_than_raising(self, cfg):
-        # Truthy-but-not-a-mapping at either level used to reach .get() on a
-        # str/list and raise AttributeError out of a function whose whole
-        # contract is "a refresh rate, or the default".
-        assert refresh_hz_from_config(cfg) == 100.0
-
-
-class TestCrispLadder:
-    """Whole-pixel speeds available on a given panel."""
-
-    def test_every_entry_is_exactly_reachable(self):
-        for c in crisp_ladder(100):
-            assert c.pixels_per_second == pytest.approx(
-                c.refresh_hz / c.frame_hold * c.pixels_per_frame)
-
-    def test_sorted_slowest_first(self):
-        speeds = [c.pixels_per_second for c in crisp_ladder(100)]
-        assert speeds == sorted(speeds)
-
-    def test_no_duplicate_speeds(self):
-        speeds = [round(c.pixels_per_second, 3) for c in crisp_ladder(100)]
-        assert len(speeds) == len(set(speeds))
-
-    def test_duplicates_resolve_to_the_smaller_step(self):
-        """100px/s is 1px every refresh or 2px every 2nd; prefer the former."""
-        entry = next(c for c in crisp_ladder(100)
-                     if c.pixels_per_second == pytest.approx(100.0))
-        assert entry.pixels_per_frame == 1
-        assert entry.frame_hold == 1
-
-    def test_ladder_scales_with_the_panel(self):
-        assert any(c.pixels_per_second == pytest.approx(60.0)
-                   for c in crisp_ladder(60))
-        assert any(c.pixels_per_second == pytest.approx(30.0)
-                   for c in crisp_ladder(60))
-
-    def test_full_refresh_speed_is_present(self):
-        for hz in (60, 75, 100, 120):
-            assert any(c.pixels_per_second == pytest.approx(float(hz))
-                       for c in crisp_ladder(hz))
-
-
-class TestSolveCrisp:
-    def test_exact_targets_are_matched_exactly(self):
-        for target in (100, 50, 25, 20):
-            assert solve_crisp(target, 100).pixels_per_second == pytest.approx(target)
-
-    def test_prefers_smooth_motion_over_raw_proximity(self):
-        """30 -> 33.3 (1px, 33fps), not 28.6 (2px at 14fps) which is nearer."""
-        got = solve_crisp(30, 100)
-        assert got.pixels_per_second == pytest.approx(33.333, abs=0.01)
-        assert got.pixels_per_frame == 1
-
-    def test_does_not_chase_a_jumpy_exact_match(self):
-        """45 -> 50 (1px, smooth) beats 42.9 (3px at 14fps)."""
-        assert solve_crisp(45, 100).pixels_per_frame == 1
-
-    def test_allows_a_two_pixel_step_when_it_is_clearly_closer(self):
-        """60 -> 66.7 (2px at 33fps) rather than 50 (1px) which is 17% slow."""
-        got = solve_crisp(60, 100)
-        assert got.pixels_per_second == pytest.approx(66.667, abs=0.01)
-        assert got.pixels_per_frame == 2
-
-    def test_result_is_always_on_the_ladder(self):
-        ladder = {round(c.pixels_per_second, 3) for c in crisp_ladder(100)}
-        for target in range(5, 205, 5):
-            assert round(solve_crisp(target, 100).pixels_per_second, 3) in ladder
-
-    def test_adapts_to_a_slower_panel(self):
-        got = solve_crisp(30, 60)
-        assert got.pixels_per_second == pytest.approx(30.0)
-        assert got.pixels_per_frame == 1
-        assert got.frame_hold == 2
-
-    def test_speeds_beyond_the_panel_clamp_to_the_fastest_available(self):
-        got = solve_crisp(10_000, 100)
-        assert got.frame_hold == 1
-        assert got.pixels_per_frame == MAX_PIXELS_PER_FRAME
-
-    def test_steppiness_labels_are_sane(self):
-        assert solve_crisp(100, 100).steppiness == "smooth"
-        assert crisp_ladder(100)[0].steppiness == "stepped"
-
-
-class TestRefreshFromDisplayManager:
-    """A plugin sees only its own config section, so the display manager is the
-    authoritative source for the panel's refresh rate."""
-
-    class DM:
-        def __init__(self, hz):
-            self.refresh_hz = hz
-            self.hold = None
-
-        def set_frame_hold(self, refreshes):
-            self.hold = refreshes
-
-    def test_uses_the_display_manager_rate(self):
-        dm = self.DM(60.0)
-        s = configure(FakeHelper(), {"display_options": {"scroll_pixels_per_second": 30.0}},
-                      display_manager=dm)
-        # 30px/s on a 60Hz panel is 1px every 2nd refresh, exactly.
-        assert s.pixels_per_second == pytest.approx(30.0)
-        assert s.frame_hold == 2
-
-    def test_explicit_refresh_hz_wins_over_the_display_manager(self):
-        dm = self.DM(60.0)
-        s = configure(FakeHelper(), {"display_options": {"scroll_pixels_per_second": 25.0}},
-                      display_manager=dm, refresh_hz=100)
-        assert s.frame_hold == 4, "25px/s at 100Hz is 1px every 4th refresh"
-
-    def test_missing_attribute_falls_back_to_the_default(self):
-        class Bare:
-            def set_frame_hold(self, refreshes):
-                self.hold = refreshes
-
-        bare = Bare()
-        s = configure(FakeHelper(), {"display_options": {"scroll_pixels_per_second": 50.0}},
-                      display_manager=bare)
-        assert s.frame_hold == 2, "assumed 100Hz"
-
-    def test_reported_settings_describe_the_display_manager_rate(self):
-        """Without snapping, nothing downstream corrects a wrong refresh rate.
-
-        The rate used to be read *after* resolve() had already filled in
-        target_fps, pixels_per_frame and the judder warning from the 100Hz
-        default -- so on a 60Hz panel every one of those described 100Hz, and
-        set_target_fps() paced the helper to 100 FPS.
-        """
-        dm = self.DM(60.0)
-        helper = FakeHelper()
-        s = configure(helper, {"display_options": {"scroll_pixels_per_second": 30.0}},
-                      display_manager=dm, snap_to_crisp=False)
-
-        assert s.target_fps == pytest.approx(60.0)
-        assert s.pixels_per_frame == pytest.approx(0.5), "30px/s over 60 frames"
-        assert helper.target_fps == pytest.approx(60.0)
-
-    def test_judder_warning_is_computed_at_the_real_refresh_rate(self):
-        dm = self.DM(60.0)
-        # 60px/s is exactly 1px per refresh at 60Hz -- crisp, no warning. At
-        # the 100Hz default it is 0.6px per refresh and would be flagged.
-        s = configure(FakeHelper(), {"display_options": {"scroll_pixels_per_second": 60.0}},
-                      display_manager=dm, snap_to_crisp=False)
-        assert s.warning is None, s.warning
-
-    def test_global_config_supplies_the_rate_without_a_display_manager(self):
-        s = configure(
-            FakeHelper(),
-            {"display_options": {"scroll_pixels_per_second": 30.0}},
-            {"display": {"hardware": {"limit_refresh_rate_hz": 60}}},
-            snap_to_crisp=False,
-        )
-        assert s.target_fps == pytest.approx(60.0)
-
-
-class TestFrameHoldIsReportedNotApplied:
-    """configure() reports the hold; the caller applies it when it scrolls.
-
-    The hold belongs to a scroll, not to a plugin's lifetime -- plugins share
-    one display manager, so one set at construction is reset the moment any
-    other plugin stops scrolling.
-    """
-
-    class RecordingDM:
-        refresh_hz = 100.0
-
-        def __init__(self):
-            self.calls = []
-
-        def set_scrolling_state(self, is_scrolling, frame_hold=1):
-            self.calls.append((is_scrolling, frame_hold))
-
-        def set_frame_hold(self, refreshes):
-            self.calls.append(("set_frame_hold", refreshes))
-
-    def test_configure_does_not_touch_the_display_manager(self):
-        dm = self.RecordingDM()
-        settings = configure(
-            FakeHelper(), {"display_options": {"scroll_pixels_per_second": 25.0}},
-            display_manager=dm)
-        assert dm.calls == [], "configure() must not apply the hold itself"
-        assert settings.frame_hold == 4, "but it must report what to apply"

test/test_scroll_helper.py

@@ -11,11 +11,7 @@ import time
 from unittest.mock import patch
 from PIL import Image
 
-from src.common.scroll_helper import (
-    ScrollHelper,
-    format_frame_stats,
-    frame_stats,
-)
+from src.common.scroll_helper import ScrollHelper
 
 
 DISPLAY_W = 64
@@ -319,77 +315,3 @@ class TestGetScrollInfo:
         helper.scroll_position = 42.0
         info = helper.get_scroll_info()
         assert info["scroll_position"] == 42.0
-
-
-class TestFrameStatsPercentiles:
-    """The stats line is the instrument this whole scroll change is measured
-    with, so its median and p95 have to be the real ones.
-
-    Both are also thresholds: stalls are counted at 1.5x the median and skips
-    at 0.5x, so an off-by-one in the median biases the counts as well as the
-    printed numbers.
-    """
-
-    # 100 samples of 1..100ms. True median 50.5ms (the mean of the two middle
-    # samples, not the upper one at 51ms); nearest-rank p95 is the 95th
-    # sample at 95ms, not the 96th at 96ms.
-    HUNDRED = [i / 1000.0 for i in range(1, 101)]
-
-    def test_even_window_median_averages_both_middle_samples(self):
-        assert frame_stats(self.HUNDRED)["median"] == pytest.approx(0.0505)
-
-    def test_p95_uses_nearest_rank(self):
-        assert frame_stats(self.HUNDRED)["p95"] == pytest.approx(0.095)
-
-    def test_odd_window_median_is_the_middle_sample(self):
-        times = [i / 1000.0 for i in range(1, 102)]  # 101 samples
-        assert frame_stats(times)["median"] == pytest.approx(0.051)
-
-    def test_single_sample_window_does_not_index_out_of_range(self):
-        stats = frame_stats([0.010])
-        assert stats["median"] == pytest.approx(0.010)
-        assert stats["p95"] == pytest.approx(0.010)
-        assert stats["min"] == stats["max"] == pytest.approx(0.010)
-
-    def test_two_sample_window(self):
-        stats = frame_stats([0.010, 0.020])
-        assert stats["median"] == pytest.approx(0.015)
-        assert stats["p95"] == pytest.approx(0.020)
-
-    def test_input_order_does_not_matter(self):
-        assert frame_stats(list(reversed(self.HUNDRED))) == frame_stats(self.HUNDRED)
-
-    def test_caller_window_is_not_mutated(self):
-        times = [0.030, 0.010, 0.020]
-        frame_stats(times)
-        assert times == [0.030, 0.010, 0.020]
-
-    def test_stall_threshold_follows_the_median(self):
-        # Ten 10ms frames and two 30ms stalls: median 10ms, so >15ms is a
-        # stall -- exactly the two.
-        stats = frame_stats([0.010] * 10 + [0.030] * 2)
-        assert stats["stalls"] == 2
-        assert stats["skips"] == 0
-
-    def test_skips_are_frames_that_never_reached_the_panel(self):
-        stats = frame_stats([0.010] * 10 + [0.002] * 3)
-        assert stats["skips"] == 3
-        assert stats["stalls"] == 0
-
-    def test_fps_is_the_reciprocal_of_the_mean(self):
-        assert frame_stats([0.010] * 50)["fps"] == pytest.approx(100.0)
-
-    def test_formatted_line_reports_the_corrected_values(self):
-        line = format_frame_stats(self.HUNDRED)
-        assert "median 50.50ms" in line, line
-        assert "p95 95.00ms" in line, line
-        assert "over 100 frames" in line, line
-
-    def test_log_frame_rate_emits_the_line_and_clears_the_window(self, helper):
-        helper._window = [0.010] * 20
-        helper.last_fps_log_time = 0.0  # force the 5s boundary
-        with patch.object(helper.logger, "info") as info:
-            helper.log_frame_rate()
-        assert info.called
-        assert "Scroll frame stats" in info.call_args[0][0]
-        assert helper._window == []

test/test_starlark_display_contract.py

@@ -1,82 +0,0 @@
-"""starlark-apps: display() must report what actually reached the panel.
-
-The display controller skips a mode only on a boolean False. Returning True
-after the frame update failed told it the mode had rendered, so it held a dead
-frame for the whole display_duration instead of rotating on -- the same class
-of defect as a display() that returns None.
-"""
-
-import sys
-from pathlib import Path
-from unittest.mock import MagicMock
-
-import pytest
-
-PLUGIN_DIR = Path(__file__).resolve().parent.parent / "plugin-repos" / "starlark-apps"
-
-
-@pytest.fixture(scope="module")
-def manager_module():
-    if not PLUGIN_DIR.exists():
-        pytest.skip("starlark-apps plugin is not checked out")
-    sys.path.insert(0, str(PLUGIN_DIR))
-    try:
-        import importlib
-        spec = importlib.util.spec_from_file_location(
-            "starlark_manager_under_test", PLUGIN_DIR / "manager.py")
-        module = importlib.util.module_from_spec(spec)
-        spec.loader.exec_module(module)
-        return module
-    except Exception as e:  # noqa: BLE001 - optional deps (pixlet, fcntl) may be absent
-        pytest.skip(f"starlark-apps manager is not importable here: {e}")
-    finally:
-        sys.path.remove(str(PLUGIN_DIR))
-
-
-def _plugin(manager_module):
-    """A manager with __init__ bypassed -- only display paths are under test."""
-    cls = manager_module.StarlarkAppsPlugin
-    inst = cls.__new__(cls)
-    inst.logger = MagicMock()
-    inst.display_manager = MagicMock()
-    inst.current_app = None
-    return inst
-
-
-class _App:
-    def __init__(self, frames):
-        self.frames = frames
-        self.current_frame_index = 0
-        self.last_frame_time = 0.0
-        self.app_id = "app"
-
-
-class TestDisplayFramePropagates:
-    def test_a_failed_update_returns_false(self, manager_module):
-        p = _plugin(manager_module)
-        p.current_app = _App([("frame", 100)])
-        p.display_manager.update_display.side_effect = RuntimeError("panel gone")
-
-        assert p._display_frame() is False
-
-    def test_a_good_update_returns_true(self, manager_module):
-        p = _plugin(manager_module)
-        p.current_app = _App([("frame", 100)])
-
-        assert p._display_frame() is True
-
-    def test_no_frames_returns_false(self, manager_module):
-        p = _plugin(manager_module)
-        p.current_app = _App([])
-
-        assert p._display_frame() is False
-
-    def test_display_reports_the_frame_failure(self, manager_module):
-        p = _plugin(manager_module)
-        p.current_app = _App([("frame", 100)])
-        p.display_manager.update_display.side_effect = RuntimeError("panel gone")
-
-        result = p.display()
-
-        assert result is False, "display() claimed success over a failed frame update"
-        assert isinstance(result, bool), "the controller only skips on a real bool"

test/web_interface/test_starlark_pixlet_routes.py

@@ -1,395 +0,0 @@
-"""The Starlark routes the frontend calls must exist.
-
-`plugins_manager.js` posts to /api/v3/starlark/install-pixlet and then reloads
-/api/v3/starlark/status. Neither route existed: #330 rewrote api_v3.py and
-dropped all thirteen Starlark routes that #253 had added, so both calls fell
-through to Flask's 404 handler, which answers
-
-    {"status": "error", "message": "Resource not found"}
-
-and the button reported "Pixlet install failed: Resource not found" -- a
-message that names neither the resource nor the cause.
-
-These assert the routes are registered and answer in the shape the frontend
-reads, so a future rewrite of this file cannot silently drop them again.
-"""
-
-import json
-import os
-from unittest.mock import MagicMock, patch
-
-import pytest
-
-
-@pytest.fixture
-def client():
-    from web_interface.app import app
-    app.config['TESTING'] = True
-    with app.test_client() as c:
-        yield c
-
-
-class TestRoutesAreRegistered:
-    """The failure was a missing route, so check the URL map directly.
-
-    All thirteen, not just the two the Pixlet button needs: #330 dropped the
-    lot, and the app store page is built on repository/browse,
-    repository/categories and repository/install, which 404 the same way.
-    """
-
-    @pytest.mark.parametrize("rule,method", [
-        ("/api/v3/starlark/install-pixlet", "POST"),
-        ("/api/v3/starlark/status", "GET"),
-        ("/api/v3/starlark/apps", "GET"),
-        ("/api/v3/starlark/upload", "POST"),
-        ("/api/v3/starlark/repository/browse", "GET"),
-        ("/api/v3/starlark/repository/categories", "GET"),
-        ("/api/v3/starlark/repository/install", "POST"),
-        ("/api/v3/starlark/apps/<app_id>", "GET"),
-        ("/api/v3/starlark/apps/<app_id>", "DELETE"),
-        ("/api/v3/starlark/apps/<app_id>/config", "GET"),
-        ("/api/v3/starlark/apps/<app_id>/config", "PUT"),
-        ("/api/v3/starlark/apps/<app_id>/toggle", "POST"),
-        ("/api/v3/starlark/apps/<app_id>/render", "POST"),
-    ])
-    def test_route_exists(self, rule, method):
-        from web_interface.app import app
-        matches = [r for r in app.url_map.iter_rules()
-                   if r.rule == rule and method in r.methods]
-        assert matches, (
-            f"{method} {rule} is not registered; the frontend calls it and "
-            "would get Flask's generic 'Resource not found'")
-
-
-class TestInstallPixlet:
-    def test_it_does_not_404(self, client):
-        with patch('web_interface.blueprints.api_v3.subprocess.run') as run:
-            run.return_value = MagicMock(returncode=0, stdout="ok", stderr="")
-            resp = client.post('/api/v3/starlark/install-pixlet')
-        assert resp.status_code != 404, "the route is still missing"
-        assert resp.get_json().get('message') != 'Resource not found'
-
-    def test_success_is_reported_in_the_shape_the_button_reads(self, client):
-        with patch('web_interface.blueprints.api_v3.subprocess.run') as run:
-            run.return_value = MagicMock(returncode=0, stdout="done", stderr="")
-            resp = client.post('/api/v3/starlark/install-pixlet')
-        body = resp.get_json()
-        assert body['status'] == 'success', body
-        assert 'message' in body, "the JS shows data.message on success"
-
-    def test_a_failed_download_says_why(self, client):
-        with patch('web_interface.blueprints.api_v3.subprocess.run') as run:
-            run.return_value = MagicMock(returncode=1, stdout="", stderr="no such release")
-            resp = client.post('/api/v3/starlark/install-pixlet')
-        body = resp.get_json()
-        assert body['status'] == 'error'
-        assert 'no such release' in body['message'], \
-            "the installer's own stderr is what tells the user what went wrong"
-
-    def test_a_timeout_is_reported_rather_than_hanging(self, client):
-        import subprocess as sp
-        with patch('web_interface.blueprints.api_v3.subprocess.run',
-                   side_effect=sp.TimeoutExpired(cmd='x', timeout=300)):
-            resp = client.post('/api/v3/starlark/install-pixlet')
-        assert resp.get_json()['status'] == 'error'
-        assert 'timed out' in resp.get_json()['message'].lower()
-
-
-class TestStarlarkStatus:
-    def test_it_does_not_404(self, client):
-        resp = client.get('/api/v3/starlark/status')
-        assert resp.status_code != 404, "the route is still missing"
-        assert resp.get_json().get('message') != 'Resource not found'
-
-    def test_it_reports_pixlet_availability_without_the_plugin_loaded(self, client):
-        # The status call runs before install too -- it must answer even when
-        # starlark-apps is not loaded, which is the state a user is in when
-        # they press the install button for the first time.
-        with patch('web_interface.blueprints.api_v3._get_starlark_plugin', return_value=None):
-            resp = client.get('/api/v3/starlark/status')
-        body = resp.get_json()
-        assert body['status'] == 'success', body
-        assert 'pixlet_available' in body
-        assert body['plugin_loaded'] is False
-
-
-class TestTheInstallerScriptIsActuallyThere:
-    def test_download_pixlet_script_exists_and_is_executable(self):
-        # install_pixlet chmods and runs this; a missing file is the one error
-        # it reports as a 404 of its own, which would look identical to the
-        # bug being fixed here.
-        import os
-        from pathlib import Path
-        from web_interface.blueprints.api_v3 import PROJECT_ROOT
-        script = Path(PROJECT_ROOT) / 'scripts' / 'download_pixlet.sh'
-        assert script.is_file(), f"{script} is missing; install_pixlet would 404"
-        assert os.access(script, os.R_OK)
-
-
-class TestTheAppStoreFlow:
-    """Browsing and installing from the Tronbyte repository.
-
-    These are the calls the app store page makes. Each returned the generic
-    "Resource not found" before this change, which is indistinguishable from
-    an empty store.
-    """
-
-    @pytest.fixture
-    def offline_repo(self):
-        """No live GitHub calls from the test suite.
-
-        browse and categories reach _get_tronbyte_repository_class() and then
-        list_all_apps_cached(); with a cold server-side cache that is a real
-        network request, which makes the run slow, rate-limitable, and able to
-        pass on a 500 because these assertions only check for a 404.
-        """
-        repo = MagicMock()
-        # Matches what the real list_all_apps_cached returns; the handler
-        # indexes every one of these keys.
-        repo.return_value.list_all_apps_cached.return_value = {
-            'apps': [{'id': 'quoteoftheday', 'name': 'A Quote A Day',
-                      'category': 'text'}],
-            'categories': ['text'],
-            'authors': ['someone'],
-            'count': 1,
-            'cached': True,
-        }
-        repo.return_value.get_rate_limit_info.return_value = {'remaining': 5000}
-        with patch('web_interface.blueprints.api_v3._get_tronbyte_repository_class',
-                   return_value=repo):
-            yield repo
-
-    def test_browse_does_not_404(self, client, offline_repo):
-        resp = client.get('/api/v3/starlark/repository/browse')
-        assert resp.status_code != 404, "the store cannot list anything"
-        assert resp.get_json().get('message') != 'Resource not found'
-
-    def test_browse_returns_the_apps_the_store_lists(self, client, offline_repo):
-        resp = client.get('/api/v3/starlark/repository/browse')
-        body = resp.get_json()
-        assert body['status'] == 'success', body
-        assert any(a.get('id') == 'quoteoftheday' for a in body.get('apps', [])), body
-
-    def test_categories_does_not_404(self, client, offline_repo):
-        resp = client.get('/api/v3/starlark/repository/categories')
-        assert resp.status_code != 404
-        assert resp.get_json().get('message') != 'Resource not found'
-
-    def test_no_live_network_call_is_made(self, client, offline_repo):
-        client.get('/api/v3/starlark/repository/browse')
-        assert offline_repo.called, \
-            "the route did not go through the patched repository class"
-
-    def test_installed_apps_list_does_not_404(self, client):
-        resp = client.get('/api/v3/starlark/apps')
-        assert resp.status_code != 404
-        assert resp.get_json().get('message') != 'Resource not found'
-
-    def test_repository_install_rejects_a_missing_body_rather_than_404ing(self, client):
-        # A 400/422 here is the route working: it received the call and said
-        # what was wrong. A 404 means it was never reached at all.
-        resp = client.post('/api/v3/starlark/repository/install',
-                           json={}, content_type='application/json')
-        assert resp.status_code != 404, "the install route is still missing"
-        assert resp.get_json().get('message') != 'Resource not found'
-
-    def test_upload_rejects_an_empty_post_rather_than_404ing(self, client):
-        resp = client.post('/api/v3/starlark/upload')
-        assert resp.status_code != 404
-        assert resp.get_json().get('message') != 'Resource not found'
-
-
-class TestNoStarlarkRouteIsMissing:
-    """A single check that the whole set is present.
-
-    #330 removed all thirteen at once by rewriting this file. One assertion
-    over the frontend's own list is what would have caught that.
-    """
-
-    def test_every_endpoint_the_frontend_calls_is_registered(self):
-        import re
-        from pathlib import Path
-        from werkzeug.exceptions import MethodNotAllowed, NotFound
-        from web_interface.app import app
-
-        root = Path(__file__).resolve().parent.parent.parent
-        js = (root / 'web_interface' / 'static' / 'v3' / 'plugins_manager.js').read_text()
-
-        # The frontend builds some of these with template literals, e.g.
-        # `/api/v3/starlark/apps/${appId}/toggle`. Substitute a placeholder so
-        # the URL is concrete, then let Werkzeug match it the way a request
-        # would -- string comparison cannot see <app_id> rules.
-        raw = set(re.findall(r"[\'\"`](/api/v3/starlark/[^\'\"`\s]*)", js))
-        urls = set()
-        for u in raw:
-            u = re.sub(r"\$\{[^}]*\}", "probe", u)
-            urls.add(u.rstrip('/') or u)
-        assert urls, "found no starlark calls in the frontend -- did the file move?"
-
-        adapter = app.url_map.bind('localhost')
-        missing = []
-        for u in sorted(urls):
-            try:
-                adapter.match(u, method='GET')
-            except MethodNotAllowed:
-                pass          # route exists, just not for GET -- fine
-            except NotFound:
-                missing.append(u)
-        assert not missing, f"the frontend calls these and they are not registered: {missing}"
-
-
-class TestInstalledAppsAppearWithTheOtherPlugins:
-    """An installed .star app must be manageable like any other plugin.
-
-    #253 surfaced installed apps in /plugins/installed as `starlark:<app_id>`
-    entries, so they could be seen and enabled/disabled from the same list as
-    everything else, and routed `starlark:` toggles to the Starlark manifest.
-    #330 removed both. The result was an app that installs successfully, then
-    appears nowhere and cannot be turned on or off.
-    """
-
-    APPS = {'apps': {'quoteoftheday': {'name': 'A Quote A Day', 'enabled': True}}}
-
-    def test_an_installed_app_is_listed(self, client):
-        with patch('web_interface.blueprints.api_v3._get_starlark_plugin', return_value=None), \
-             patch('web_interface.blueprints.api_v3._read_starlark_manifest', return_value=self.APPS):
-            resp = client.get('/api/v3/plugins/installed')
-        ids = [p['id'] for p in resp.get_json()['data']['plugins']]
-        assert 'starlark:quoteoftheday' in ids, \
-            "an installed Starlark app does not appear among the plugins"
-
-    def test_the_entry_carries_what_the_ui_needs(self, client):
-        with patch('web_interface.blueprints.api_v3._get_starlark_plugin', return_value=None), \
-             patch('web_interface.blueprints.api_v3._read_starlark_manifest', return_value=self.APPS):
-            resp = client.get('/api/v3/plugins/installed')
-        entry = next(p for p in resp.get_json()['data']['plugins']
-                     if p['id'] == 'starlark:quoteoftheday')
-        assert entry['name'] == 'A Quote A Day'
-        assert entry['enabled'] is True
-        assert entry['is_starlark_app'] is True, "the UI keys its Starlark handling off this"
-        assert entry['category'] == 'Starlark App'
-
-    def test_a_starlark_failure_does_not_empty_the_plugin_list(self, client):
-        # The virtual entries are appended to the real ones; a broken manifest
-        # must cost the Starlark rows, not everybody else's.
-        with patch('web_interface.blueprints.api_v3._get_starlark_plugin',
-                   side_effect=RuntimeError('boom')):
-            resp = client.get('/api/v3/plugins/installed')
-        assert resp.status_code == 200
-        assert resp.get_json()['status'] == 'success'
-
-    def test_toggling_an_app_does_not_report_plugin_not_found(self, client):
-        written = {}
-        with patch('web_interface.blueprints.api_v3._get_starlark_plugin', return_value=None), \
-             patch('web_interface.blueprints.api_v3._read_starlark_manifest',
-                   return_value={'apps': {'quoteoftheday': {'enabled': True}}}), \
-             patch('web_interface.blueprints.api_v3._write_starlark_manifest',
-                   side_effect=lambda m: written.update(m) or True):
-            resp = client.post('/api/v3/plugins/toggle',
-                               json={'plugin_id': 'starlark:quoteoftheday', 'enabled': False})
-        body = resp.get_json()
-        assert body['status'] == 'success', body
-        assert body['enabled'] is False
-        assert written['apps']['quoteoftheday']['enabled'] is False, \
-            "the manifest was not actually updated"
-
-    def test_toggling_an_unknown_app_says_so(self, client):
-        with patch('web_interface.blueprints.api_v3._get_starlark_plugin', return_value=None), \
-             patch('web_interface.blueprints.api_v3._read_starlark_manifest',
-                   return_value={'apps': {}}):
-            resp = client.post('/api/v3/plugins/toggle',
-                               json={'plugin_id': 'starlark:nope', 'enabled': True})
-        assert resp.status_code == 404
-        assert 'nope' in resp.get_json()['message']
-
-    def test_a_traversal_app_id_is_rejected_before_touching_the_manifest(self, client):
-        resp = client.post('/api/v3/plugins/toggle',
-                           json={'plugin_id': 'starlark:../../etc/passwd', 'enabled': True})
-        assert resp.status_code == 400
-        assert 'traversal' in resp.get_json()['message']
-
-    def test_an_app_id_the_listing_published_can_be_toggled(self, client):
-        """The id here is exactly what _starlark_virtual_plugins publishes.
-
-        It used to be re-slugified on the way back in -- lowercased, with every
-        character outside [a-z0-9_] replaced -- so an app stored as 'My-App'
-        was listed as 'starlark:My-App' and looked up as 'my_app'. Toggling an
-        app the page had just drawn answered 404.
-        """
-        manifest = {'apps': {'My-App': {'name': 'My App', 'enabled': False}}}
-        with patch('web_interface.blueprints.api_v3._get_starlark_plugin', return_value=None), \
-             patch('web_interface.blueprints.api_v3._read_starlark_manifest',
-                   return_value=manifest), \
-             patch('web_interface.blueprints.api_v3._write_starlark_manifest',
-                   return_value=True) as write:
-            resp = client.post('/api/v3/plugins/toggle',
-                               json={'plugin_id': 'starlark:My-App', 'enabled': True})
-        assert resp.status_code == 200, resp.get_json()
-        assert write.called, "the toggle never reached the manifest"
-        assert manifest['apps']['My-App']['enabled'] is True
-
-    def test_a_loaded_app_missing_from_the_manifest_does_not_500(self, client):
-        """_update_manifest_safe does not catch KeyError, so indexing an entry
-        that is not on disk yet escaped as a 500 instead of writing it."""
-        app = MagicMock()
-        app.manifest = {'enabled': False}
-        plugin = MagicMock()
-        plugin.apps = {'demo': app}
-        written = {}
-
-        def run_updater(fn):
-            fn(written)
-            return True
-
-        plugin._update_manifest_safe.side_effect = run_updater
-
-        with patch('web_interface.blueprints.api_v3._get_starlark_plugin', return_value=plugin):
-            resp = client.post('/api/v3/plugins/toggle',
-                               json={'plugin_id': 'starlark:demo', 'enabled': True})
-        assert resp.status_code == 200, resp.get_json()
-        assert written['apps']['demo']['enabled'] is True
-
-
-class TestTheManifestStaysRelocatable:
-    """`star_file` is joined to the app's own directory by its readers.
-
-    _standalone_render_starlark_app does `app_dir / app_data.get('star_file',
-    f'{app_id}.star')`, so the key's default is a bare filename. Storing an
-    absolute path gave it a second meaning, and Path.__truediv__ discards the
-    left side when the right is absolute -- which pinned the manifest to the
-    PROJECT_ROOT that installed it.
-    """
-
-    @pytest.fixture
-    def starlark_dir(self, tmp_path, monkeypatch):
-        from web_interface.blueprints import api_v3 as module
-        apps_dir = tmp_path / "starlark-apps"
-        apps_dir.mkdir()
-        monkeypatch.setattr(module, '_STARLARK_APPS_DIR', apps_dir)
-        monkeypatch.setattr(module, '_STARLARK_MANIFEST_FILE', apps_dir / 'manifest.json')
-        return apps_dir
-
-    def _install(self, tmp_path):
-        from web_interface.blueprints import api_v3 as module
-        source = tmp_path / "source.star"
-        source.write_text("# app")
-        with patch.object(module, '_get_pixlet_renderer_class',
-                          side_effect=ImportError("no pixlet here")):
-            assert module._install_star_file('demo', str(source), {'name': 'Demo'})
-        return json.loads((module._STARLARK_MANIFEST_FILE).read_text())['apps']['demo']
-
-    def test_the_star_file_is_recorded_by_name(self, starlark_dir, tmp_path):
-        assert self._install(tmp_path)['star_file'] == 'demo.star'
-
-    def test_the_recorded_path_is_not_absolute(self, starlark_dir, tmp_path):
-        """An absolute value survives a move only by accident."""
-        assert not os.path.isabs(self._install(tmp_path)['star_file'])
-
-    def test_the_value_resolves_against_the_app_directory(self, starlark_dir, tmp_path):
-        """Which is the one thing every reader of this key does with it."""
-        entry = self._install(tmp_path)
-        assert (starlark_dir / 'demo' / entry['star_file']).is_file()
-
-    def test_it_matches_the_default_a_reader_falls_back_to(self, starlark_dir, tmp_path):
-        """Stored and defaulted values must mean the same thing."""
-        assert self._install(tmp_path)['star_file'] == 'demo.star'

web_interface/app.py

@@ -83,11 +83,11 @@ plugin_system_config = config.get('plugin_system', {})
 plugins_dir_name = plugin_system_config.get('plugins_directory', 'plugin-repos')
 
 # Resolve plugin directory - handle both absolute and relative paths
+project_root = Path(__file__).parent.parent
 if os.path.isabs(plugins_dir_name):
     plugins_dir = Path(plugins_dir_name)
 else:
     # If relative, resolve relative to the project root (LEDMatrix directory)
-    project_root = Path(__file__).parent.parent
     plugins_dir = project_root / plugins_dir_name
 
 plugin_manager = PluginManager(
@@ -157,6 +157,7 @@ _health_monitor_initialized = False
 # Register blueprints
 from web_interface.blueprints.pages_v3 import pages_v3
 from web_interface.blueprints.api_v3 import api_v3
+from web_interface.blueprints.composer import composer_bp
 
 # Initialize managers in blueprints
 pages_v3.config_manager = config_manager
@@ -173,6 +174,11 @@ api_v3.operation_queue = operation_queue
 api_v3.plugin_state_manager = plugin_state_manager
 api_v3.operation_history = operation_history
 api_v3.health_monitor = health_monitor
+
+composer_bp.config_manager = config_manager
+composer_bp.plugin_manager = plugin_manager
+composer_bp.plugins_dir = plugins_dir
+composer_bp.project_root = project_root
 # Initialize cache manager for API endpoints
 from src.cache_manager import CacheManager
 api_v3.cache_manager = CacheManager()
@@ -200,6 +206,7 @@ except Exception as _hm_err:  # pragma: no cover - defensive startup guard
 app.register_blueprint(pages_v3, url_prefix='')
 app.register_blueprint(pages_v3, url_prefix='/v3', name='pages_v3_legacy')
 app.register_blueprint(api_v3, url_prefix='/api/v3')
+app.register_blueprint(composer_bp, url_prefix='/composer')
 
 # Route to serve plugin asset files (registered on main app, not blueprint, for /assets/... path)
 @app.route('/assets/plugins/<plugin_id>/uploads/<path:filename>', methods=['GET'])

web_interface/static/v3/app.css

@@ -426,6 +426,9 @@ a, button, input, select, textarea {
     .md\:hidden { display: none; }
     .md\:block { display: block; }
     .md\:w-auto { width: auto; }
+    /* composer.html labels its toolbar buttons `hidden md:inline`, so without
+       this the label is hidden at every width and the buttons stay icon-only. */
+    .md\:inline { display: inline; }
 }
 
 @media (min-width: 1024px) {

web_interface/static/v3/js/composer/composer-canvas.js

@@ -0,0 +1,817 @@
+/**
+ * ComposerCanvas — stateless LED matrix canvas renderer.
+ *
+ * Coordinate system: LED pixels (integers). All drawing multiplies by SCALE.
+ * PIL draw.text(x,y) is top-left; canvas fillText(x,y) is baseline.
+ *   → Canvas text cy = (actualY + fontSizePx) * SCALE
+ *
+ * Anchors: element x/y are offsets from their anchor point:
+ *   xAnchor=null/'left'  → x is fixed offset from left
+ *   xAnchor='center'     → x is offset from width/2
+ *   xAnchor='right'      → x is offset inward from right edge
+ *   yAnchor follows the same pattern with 'top'/'middle'/'bottom'
+ *
+ * Breakpoints: elements with minWidth > currentMatrixW are rendered at 25% opacity.
+ *
+ * Resize handles: drawn on selected rectangles; 8 handles (corners + edge mids).
+ */
+/* eslint-disable security/detect-object-injection --
+   Every hit in this file is either an array index (this.elements[idx],
+   rawVals[i]) or a lookup on a module-private map keyed by an internal
+   element type (_HANDLE_CURSORS, ELEMENT_DEFAULTS, previewValues). None of
+   them takes an attacker-supplied property name, so none can reach a
+   prototype. Disabled per file rather than eight times inline. */
+window.ComposerCanvas = (() => {
+  'use strict';
+
+  let _canvas = null;
+  let _ctx = null;
+  let _showGrid = true;
+
+  //: Element types the canvas draws resize handles for. Exported because the
+  //: editor has to gate its resize and hover behaviour on exactly this list --
+  //: the two had drifted, so handles appeared on five shapes that could not
+  //: actually be resized.
+  const RESIZABLE_TYPES = ['rectangle', 'rounded_rectangle', 'ellipse', 'arc',
+                           'gauge', 'sparkline'];
+
+  const DISPLAY_PRESETS = [
+    { label: '64×32',   w: 64,  h: 32 },
+    { label: '128×32',  w: 128, h: 32 },
+    { label: '128×64',  w: 128, h: 64 },
+    { label: '256×32',  w: 256, h: 32 },
+    { label: '256×64',  w: 256, h: 64 },
+  ];
+
+  const FONT_MAP = {
+    press_start:   { family: "'PressStart2P', monospace", sizePx: 8,  charW: 8 },
+    four_by_six:   { family: 'monospace',                sizePx: 6,  charW: 4 },
+    five_by_seven: { family: 'monospace',                sizePx: 7,  charW: 5 },
+  };
+
+  const ELEMENT_DEFAULTS = {
+    text: {
+      text: 'Hello', font: 'press_start',
+      r: 255, g: 255, b: 255,
+      text2: '', lineSpacing: 2, textAlign: 'left',
+      xAnchor: null, yAnchor: null, minWidth: 0, locked: false, blink: false, visible: true,
+    },
+    dynamic_text: {
+      binding: { source: 'config', key: '', format: null },
+      font: 'press_start', textAlign: 'left',
+      r: 255, g: 200, b: 100,
+      xAnchor: null, yAnchor: null, minWidth: 0, locked: false, blink: false, visible: true,
+    },
+    clock: {
+      format: '%H:%M', font: 'press_start',
+      r: 100, g: 255, b: 100,
+      format2: '', lineSpacing: 2, textAlign: 'left',
+      xAnchor: null, yAnchor: null, minWidth: 0, locked: false, blink: false, visible: true,
+    },
+    rectangle: {
+      width: 20, height: 8,
+      fillR: 0, fillG: 0, fillB: 128, hasFill: true,
+      outR: 255, outG: 255, outB: 255, hasOutline: true,
+      xAnchor: null, yAnchor: null, minWidth: 0, locked: false, blink: false, visible: true,
+    },
+    ellipse: {
+      width: 24, height: 12,
+      fillR: 0, fillG: 100, fillB: 200, hasFill: true,
+      outR: 100, outG: 180, outB: 255, hasOutline: true,
+      xAnchor: null, yAnchor: null, minWidth: 0, locked: false, blink: false, visible: true,
+    },
+    arc: {
+      width: 24, height: 24,
+      startAngle: 0, endAngle: 270, lineWidth: 2,
+      r: 255, g: 200, b: 0,
+      xAnchor: null, yAnchor: null, minWidth: 0, locked: false, blink: false, visible: true,
+    },
+    pixel: {
+      r: 255, g: 255, b: 255,
+      xAnchor: null, yAnchor: null, minWidth: 0, locked: false, blink: false, visible: true,
+    },
+    rounded_rectangle: {
+      width: 24, height: 10, borderRadius: 3,
+      fillR: 0, fillG: 80, fillB: 180, hasFill: true,
+      outR: 120, outG: 180, outB: 255, hasOutline: true,
+      xAnchor: null, yAnchor: null, minWidth: 0, locked: false, blink: false, visible: true,
+    },
+    line: {
+      x0: 0, y0: 16, x1: 63, y1: 16,
+      r: 180, g: 180, b: 180, lineWidth: 1,
+      xAnchor: null, yAnchor: null, minWidth: 0, locked: false, blink: false, visible: true,
+    },
+    divider: {
+      orientation: 'horizontal', y: 16, x: 64,
+      r: 100, g: 100, b: 100,
+      xAnchor: null, yAnchor: null, minWidth: 0, locked: false, blink: false, visible: true,
+    },
+    progress_bar: {
+      barWidth: 60, barHeight: 6,
+      binding: { source: 'config', key: '', format: null },
+      r: 80, g: 200, b: 80,
+      bgR: 30, bgG: 30, bgB: 30, hasBg: true,
+      outR: 100, outG: 100, outB: 100, hasOutline: true,
+      previewPct: 65,
+      xAnchor: null, yAnchor: null, minWidth: 0, locked: false, blink: false, visible: true,
+    },
+    countdown: {
+      binding: { source: 'config', key: '', format: null },
+      countdownFormat: 'dh',
+      font: 'four_by_six', textAlign: 'left',
+      r: 255, g: 180, b: 0,
+      previewText: '42d 3h',
+      xAnchor: null, yAnchor: null, minWidth: 0, locked: false, blink: false, visible: true,
+    },
+    marquee: {
+      text: 'Scrolling text', font: 'press_start',
+      r: 255, g: 255, b: 255,
+      scrollSpeed: 1, gap: 16, direction: 'left',
+      xAnchor: null, yAnchor: null, minWidth: 0, locked: false, blink: false, visible: true,
+    },
+    section: {
+      label: 'Section',
+      xAnchor: null, yAnchor: null, minWidth: 0, locked: false, blink: false, visible: true,
+    },
+    pips: {
+      count: 5, filled: 3, pipSize: 4, pipSpacing: 2,
+      r: 255, g: 200, b: 0,
+      emptyR: 50, emptyG: 50, emptyB: 50, showEmpty: true,
+      binding: { source: 'config', key: '', format: null },
+      xAnchor: null, yAnchor: null, minWidth: 0, locked: false, blink: false, visible: true,
+    },
+    sparkline: {
+      width: 40, height: 12,
+      barCount: 8, barSpacing: 1,
+      r: 80, g: 200, b: 120,
+      bgR: 30, bgG: 30, bgB: 30, hasBg: false,
+      binding: { source: 'config', key: '', format: null },
+      previewData: '0.3,0.6,0.4,0.8,0.5,0.9,0.7,0.85',
+      xAnchor: null, yAnchor: null, minWidth: 0, locked: false, blink: false, visible: true,
+    },
+    gauge: {
+      width: 32, height: 32,
+      startAngle: 135, endAngle: 45, lineWidth: 3,
+      binding: { source: 'config', key: '', format: null },
+      r: 80, g: 220, b: 80,
+      trackR: 40, trackG: 40, trackB: 40, hasTrack: true,
+      showLabel: true, font: 'four_by_six', labelR: 200, labelG: 200, labelB: 200,
+      previewPct: 65,
+      xAnchor: null, yAnchor: null, minWidth: 0, locked: false, blink: false, visible: true,
+    },
+  };
+
+  // ── Anchor resolution ────────────────────────────────────────────────
+  function resolveAnchor(val, anchor, dim) {
+    if (!anchor || anchor === 'left' || anchor === 'top') return val;
+    if (anchor === 'center' || anchor === 'middle') return Math.floor(dim / 2) + val;
+    if (anchor === 'right' || anchor === 'bottom') return dim - val;
+    return val;
+  }
+
+  function computeActualPos(el, matrixW, matrixH) {
+    const ax = resolveAnchor(el.x ?? el.x0 ?? 0, el.xAnchor, matrixW);
+    const ay = resolveAnchor(el.y ?? el.y0 ?? 0, el.yAnchor, matrixH);
+    return { x: ax, y: ay };
+  }
+
+  // ── Bounding box (LED pixel space) ──────────────────────────────────
+  function getBoundingBox(el, matrixW, matrixH) {
+    const { x: ax, y: ay } = computeActualPos(el, matrixW, matrixH);
+    const finfo = FONT_MAP[el.font] || FONT_MAP.press_start;
+
+    switch (el.type) {
+      case 'text': {
+        const t1 = el.text || '', t2 = el.text2 || '';
+        const w = Math.max(t1.length, t2.length) * finfo.charW;
+        const h = t2 ? finfo.sizePx * 2 + (el.lineSpacing ?? 2) : finfo.sizePx;
+        const bx = el.textAlign === 'center' ? ax - w / 2 : el.textAlign === 'right' ? ax - w : ax;
+        return { x: bx, y: ay, w, h };
+      }
+      case 'dynamic_text': {
+        const key = el.binding?.key || '?';
+        const w = (`{${key}}`).length * finfo.charW;
+        const bx = el.textAlign === 'center' ? ax - w / 2 : el.textAlign === 'right' ? ax - w : ax;
+        return { x: bx, y: ay, w, h: finfo.sizePx };
+      }
+      case 'clock': {
+        const t1 = el.format || '%H:%M', t2 = el.format2 || '';
+        const w = Math.max(t1.length, t2.length) * finfo.charW;
+        const h = t2 ? finfo.sizePx * 2 + (el.lineSpacing ?? 2) : finfo.sizePx;
+        const bx = el.textAlign === 'center' ? ax - w / 2 : el.textAlign === 'right' ? ax - w : ax;
+        return { x: bx, y: ay, w, h };
+      }
+      case 'countdown': {
+        const pt = el.previewText || '--d --h';
+        const w = pt.length * finfo.charW;
+        const bx = el.textAlign === 'center' ? ax - w / 2 : el.textAlign === 'right' ? ax - w : ax;
+        return { x: bx, y: ay, w, h: finfo.sizePx };
+      }
+      case 'rectangle':
+      case 'rounded_rectangle':
+      case 'ellipse':
+      case 'arc':
+        return { x: ax, y: ay, w: el.width, h: el.height };
+      case 'pixel':
+        return { x: ax, y: ay, w: 1, h: 1 };
+      case 'line': {
+        // Same anchor offset the draw path applies, or the hit box sits where
+        // the line used to be.
+        const dx = ax - el.x0, dy = ay - el.y0;
+        return {
+          x: Math.min(el.x0, el.x1) + dx, y: Math.min(el.y0, el.y1) + dy,
+          w: Math.max(1, Math.abs(el.x1 - el.x0)),
+          h: Math.max(1, Math.abs(el.y1 - el.y0)),
+        };
+      }
+      case 'divider':
+        return el.orientation === 'horizontal'
+          ? { x: 0,  y: ay, w: matrixW, h: 1 }
+          : { x: ax, y: 0,  w: 1,       h: matrixH };
+      case 'progress_bar':
+        return { x: ax, y: ay, w: el.barWidth ?? 60, h: el.barHeight ?? 6 };
+      case 'marquee': {
+        const mfinfo = FONT_MAP[el.font] || FONT_MAP.press_start;
+        return { x: 0, y: ay, w: matrixW, h: mfinfo.sizePx };
+      }
+      case 'gauge':
+        return { x: ax, y: ay, w: el.width ?? 32, h: el.height ?? 32 };
+      case 'sparkline':
+        return { x: ax, y: ay, w: el.width ?? 40, h: el.height ?? 12 };
+      case 'pips': {
+        const pc = el.count ?? 5, ps = el.pipSize ?? 4, pg = el.pipSpacing ?? 2;
+        return { x: ax, y: ay, w: pc * ps + (pc - 1) * pg, h: ps };
+      }
+      case 'section': {
+        // Was 0x0, so the element was unselectable except through the 3px
+        // hit-test padding and drew nothing at all -- a user adding one from
+        // the palette saw an empty canvas.
+        // Same font resolution as the draw case below, or the box will not
+        // match the glyphs: getBoundingBox's shared `finfo` falls back to
+        // press_start, and a section has no font of its own.
+        const sinfo = FONT_MAP[el.font] || FONT_MAP.four_by_six;
+        const label = el.label || 'Section';
+        return { x: ax, y: ay, w: label.length * sinfo.charW, h: sinfo.sizePx };
+      }
+      default:
+        return { x: ax, y: ay, w: 4, h: 4 };
+    }
+  }
+
+  // ── Resize handle support ─────────────────────────────────────────────
+  // Returns 8 handle points for a rectangle in LED pixel space
+  function _getRectHandles(el, matrixW, matrixH) {
+    const { x: ax, y: ay } = computeActualPos(el, matrixW, matrixH);
+    const w = el.width, h = el.height;
+    const cx = ax + w / 2, cy = ay + h / 2;
+    return {
+      nw: { x: ax,      y: ay      },
+      n:  { x: cx,      y: ay      },
+      ne: { x: ax + w,  y: ay      },
+      w:  { x: ax,      y: cy      },
+      e:  { x: ax + w,  y: cy      },
+      sw: { x: ax,      y: ay + h  },
+      s:  { x: cx,      y: ay + h  },
+      se: { x: ax + w,  y: ay + h  },
+    };
+  }
+
+  // Returns the handle direction under LED-space point (lx, ly), or null
+  function getResizeHandle(el, lx, ly, matrixW, matrixH) {
+    if (!RESIZABLE_TYPES.includes(el.type)) return null;
+    const handles = _getRectHandles(el, matrixW, matrixH);
+    const PAD = 4;
+    for (const [dir, pt] of Object.entries(handles)) {
+      if (Math.abs(lx - pt.x) <= PAD && Math.abs(ly - pt.y) <= PAD) return dir;
+    }
+    return null;
+  }
+
+  const _HANDLE_CURSORS = {
+    nw: 'nw-resize', n: 'n-resize', ne: 'ne-resize',
+    w: 'w-resize', e: 'e-resize',
+    sw: 'sw-resize', s: 's-resize', se: 'se-resize',
+  };
+  function getCursorForHandle(handle) {
+    return _HANDLE_CURSORS[handle] || 'crosshair';
+  }
+
+  // ── Hit test ─────────────────────────────────────────────────────────
+  function hitTest(el, lx, ly, matrixW, matrixH) {
+    const PAD = 3;
+    const bb = getBoundingBox(el, matrixW, matrixH);
+    return (
+      lx >= bb.x - PAD && lx <= bb.x + bb.w + PAD &&
+      ly >= bb.y - PAD && ly <= bb.y + bb.h + PAD
+    );
+  }
+
+  // ── Draw a single element ─────────────────────────────────────────────
+  function _drawElement(ctx, el, SCALE, matrixW, matrixH, opts = {}) {
+    const s = SCALE;
+    const { x: ax, y: ay } = computeActualPos(el, matrixW, matrixH);
+    const belowBreakpoint = el.minWidth > 0 && matrixW < el.minWidth;
+    const hidden = el.visible === false;
+
+    ctx.save();
+    if (hidden) ctx.globalAlpha = 0.12;
+    else if (belowBreakpoint) ctx.globalAlpha = 0.25;
+
+    // Blink animation: when blinkOff, fully hide blinking elements
+    if (el.blink) {
+      if (opts.blinkOff) { ctx.restore(); return; }
+      ctx.globalAlpha *= 0.55;
+    }
+
+    // Helper: compute draw X for text alignment
+    // biome-ignore lint/correctness/useQwikValidLexicalScope: not Qwik -- this is an Alpine.js component, and the rule is about Qwik's $() serialization boundary, which does not exist here.
+    const _textX = (text, finfo) => {
+      const tw = text.length * finfo.charW * s;
+      if (el.textAlign === 'center') return ax * s - tw / 2;
+      if (el.textAlign === 'right')  return ax * s - tw;
+      return ax * s;
+    };
+
+    try {
+      switch (el.type) {
+        case 'section': {
+          // A design-time label: it marks a region for the author and is not
+          // emitted into the generated plugin. There was no case here at all,
+          // so adding "Section Label" from the palette drew nothing and left
+          // the user with an apparently broken control.
+          const sfinfo = FONT_MAP[el.font] || FONT_MAP.four_by_six;
+          ctx.font = `${sfinfo.sizePx * s}px ${sfinfo.family}`;
+          ctx.fillStyle = `rgba(${el.r ?? 120},${el.g ?? 120},${el.b ?? 120},0.85)`;
+          ctx.textBaseline = 'top';
+          ctx.fillText(el.label || 'Section', ax * s, ay * s);
+          break;
+        }
+        case 'text':
+        case 'dynamic_text':
+        case 'clock': {
+          const finfo = FONT_MAP[el.font] || FONT_MAP.press_start;
+          const key = el.binding?.key || '?';
+          const pv = opts.previewValues?.[key];
+          // Substitute {variable} tokens in text using previewValues
+          // biome-ignore lint/correctness/useQwikValidLexicalScope: not Qwik -- this is an Alpine.js component, and the rule is about Qwik's $() serialization boundary, which does not exist here.
+          const _subVars = str => (str || '').replace(/\{(\w+)\}/g, (_, k) => {
+            const v = opts.previewValues?.[k];
+            return v !== undefined && v !== '' ? String(v) : `{${k}}`;
+          });
+          const displayText =
+            el.type === 'text'    ? _subVars(el.text || '')
+            : el.type === 'clock' ? (el.format || '%H:%M')
+            : (pv !== undefined && pv !== '' ? String(pv) : `{${key}}`);
+          ctx.font = `${finfo.sizePx * s}px ${finfo.family}`;
+          ctx.fillStyle = `rgb(${el.r},${el.g},${el.b})`;
+          ctx.fillText(displayText, _textX(displayText, finfo), (ay + finfo.sizePx) * s);
+          // Second line (text and clock)
+          if (el.type === 'text' && el.text2) {
+            const t2 = _subVars(el.text2);
+            const y2 = ay + finfo.sizePx + (el.lineSpacing ?? 2);
+            ctx.fillText(t2, _textX(t2, finfo), (y2 + finfo.sizePx) * s);
+          }
+          if (el.type === 'clock' && el.format2) {
+            const y2 = ay + finfo.sizePx + (el.lineSpacing ?? 2);
+            ctx.fillText(el.format2, _textX(el.format2, finfo), (y2 + finfo.sizePx) * s);
+          }
+          break;
+        }
+
+        case 'countdown': {
+          const finfo = FONT_MAP[el.font] || FONT_MAP.press_start;
+          const t = el.previewText || '--d --h';
+          ctx.font = `${finfo.sizePx * s}px ${finfo.family}`;
+          ctx.fillStyle = `rgb(${el.r},${el.g},${el.b})`;
+          ctx.fillText(t, _textX(t, finfo), (ay + finfo.sizePx) * s);
+          break;
+        }
+
+        case 'rectangle': {
+          const rx = ax * s, ry = ay * s;
+          const rw = el.width * s, rh = el.height * s;
+          if (el.hasFill) {
+            ctx.fillStyle = `rgb(${el.fillR},${el.fillG},${el.fillB})`;
+            ctx.fillRect(rx, ry, rw, rh);
+          }
+          if (el.hasOutline) {
+            ctx.strokeStyle = `rgb(${el.outR},${el.outG},${el.outB})`;
+            // 1 LED pixel, not 1 canvas pixel: at SCALE>1 an unscaled stroke
+            // renders thinner than the geometry it outlines.
+            ctx.lineWidth = s;
+            ctx.strokeRect(rx, ry, rw, rh);
+          }
+          break;
+        }
+
+        case 'ellipse': {
+          const cx = (ax + el.width / 2) * s;
+          const cy = (ay + el.height / 2) * s;
+          const lwPx = s;  // 1 LED pixel
+          // Inset by half the stroke, which straddles the path, so the outline
+          // stays within the element's bounds.
+          const rx = Math.max(0, (el.width / 2) * s - (el.hasOutline ? lwPx / 2 : 0));
+          const ry = Math.max(0, (el.height / 2) * s - (el.hasOutline ? lwPx / 2 : 0));
+          ctx.beginPath();
+          ctx.ellipse(cx, cy, rx, ry, 0, 0, Math.PI * 2);
+          if (el.hasFill) {
+            ctx.fillStyle = `rgb(${el.fillR},${el.fillG},${el.fillB})`;
+            ctx.fill();
+          }
+          if (el.hasOutline) {
+            ctx.strokeStyle = `rgb(${el.outR},${el.outG},${el.outB})`;
+            ctx.lineWidth = lwPx;
+            ctx.stroke();
+          }
+          break;
+        }
+
+        case 'arc': {
+          const cx = (ax + el.width / 2) * s;
+          const cy = (ay + el.height / 2) * s;
+          const lwPx = Math.max(1, el.lineWidth || 2) * s;
+          const rx = Math.max(0, (el.width / 2) * s - lwPx / 2);
+          const ry = Math.max(0, (el.height / 2) * s - lwPx / 2);
+          // PIL: 0°=right, clockwise. Canvas: same with anticlockwise=false
+          const startRad = (el.startAngle ?? 0) * Math.PI / 180;
+          const endRad = (el.endAngle ?? 270) * Math.PI / 180;
+          ctx.beginPath();
+          ctx.ellipse(cx, cy, rx, ry, 0, startRad, endRad, false);
+          ctx.strokeStyle = `rgb(${el.r},${el.g},${el.b})`;
+          ctx.lineWidth = lwPx;
+          ctx.stroke();
+          break;
+        }
+
+        case 'pixel': {
+          ctx.fillStyle = `rgb(${el.r},${el.g},${el.b})`;
+          ctx.fillRect(ax * s, ay * s, s, s);
+          break;
+        }
+
+        case 'rounded_rectangle': {
+          const rx = ax * s, ry = ay * s;
+          const rw = el.width * s, rh = el.height * s;
+          const rad = Math.min((el.borderRadius ?? 3) * s, rw / 2, rh / 2);
+          ctx.beginPath();
+          ctx.roundRect(rx, ry, rw, rh, rad);
+          if (el.hasFill) {
+            ctx.fillStyle = `rgb(${el.fillR},${el.fillG},${el.fillB})`;
+            ctx.fill();
+          }
+          if (el.hasOutline) {
+            ctx.strokeStyle = `rgb(${el.outR},${el.outG},${el.outB})`;
+            ctx.lineWidth = s;
+            ctx.stroke();
+          }
+          break;
+        }
+
+        case 'line': {
+          // ax/ay resolve from el.x0 for a line, so this is the anchor offset.
+          // Without it a line stayed put while every other type moved.
+          const dx = ax - el.x0, dy = ay - el.y0;
+          ctx.strokeStyle = `rgb(${el.r},${el.g},${el.b})`;
+          ctx.lineWidth = Math.max(1, el.lineWidth || 1) * s;
+          ctx.beginPath();
+          ctx.moveTo((el.x0 + dx) * s, (el.y0 + dy) * s);
+          ctx.lineTo((el.x1 + dx) * s, (el.y1 + dy) * s);
+          ctx.stroke();
+          break;
+        }
+
+        case 'divider': {
+          const isH = (el.orientation || 'horizontal') === 'horizontal';
+          ctx.strokeStyle = `rgb(${el.r},${el.g},${el.b})`;
+          ctx.lineWidth = s;
+          ctx.beginPath();
+          if (isH) {
+            ctx.moveTo(0, (ay + 0.5) * s);
+            ctx.lineTo(_canvas.width, (ay + 0.5) * s);
+          } else {
+            ctx.moveTo((ax + 0.5) * s, 0);
+            ctx.lineTo((ax + 0.5) * s, _canvas.height);
+          }
+          ctx.stroke();
+          break;
+        }
+
+        case 'pips': {
+          const pipCount = Math.max(1, el.count ?? 5);
+          const pvPips = opts.previewValues?.[el.binding?.key];
+          const filledN = pvPips !== undefined
+            ? Math.max(0, Math.min(pipCount, Math.round(parseFloat(pvPips) || 0)))
+            : Math.max(0, Math.min(pipCount, el.filled ?? 3));
+          const ps = Math.max(1, el.pipSize ?? 4);
+          const pg = Math.max(0, el.pipSpacing ?? 2);
+          for (let i = 0; i < pipCount; i++) {
+            const isFilled = i < filledN;
+            if (!isFilled && !el.showEmpty) continue;
+            ctx.fillStyle = isFilled
+              ? `rgb(${el.r},${el.g},${el.b})`
+              : `rgb(${el.emptyR ?? 50},${el.emptyG ?? 50},${el.emptyB ?? 50})`;
+            ctx.fillRect((ax + i * (ps + pg)) * s, ay * s, ps * s, ps * s);
+          }
+          break;
+        }
+
+        case 'sparkline': {
+          const slW = el.width ?? 40, slH = el.height ?? 12;
+          const count = Math.max(1, el.barCount ?? 8);
+          const spacing = el.barSpacing ?? 1;
+          const barW = Math.max(1, Math.floor((slW - spacing * (count - 1)) / count));
+          const rawVals = (el.previewData || '').split(',')
+            .map(v => parseFloat(v.trim())).filter(n => !isNaN(n));
+          while (rawVals.length < count) rawVals.push(0);
+          const maxV = Math.max(...rawVals.slice(0, count), 0.001);
+          const rx = ax * s, ry = ay * s;
+          if (el.hasBg) {
+            ctx.fillStyle = `rgb(${el.bgR ?? 30},${el.bgG ?? 30},${el.bgB ?? 30})`;
+            ctx.fillRect(rx, ry, slW * s, slH * s);
+          }
+          ctx.fillStyle = `rgb(${el.r},${el.g},${el.b})`;
+          for (let i = 0; i < count; i++) {
+            const norm = Math.max(0, Math.min(1, rawVals[i] / maxV));
+            const barH = Math.max(1, Math.round(slH * norm));
+            const bx = rx + (barW + spacing) * i * s;
+            const by = ry + (slH - barH) * s;
+            ctx.fillRect(bx, by, barW * s, barH * s);
+          }
+          break;
+        }
+
+        case 'gauge': {
+          const gw = (el.width ?? 32), gh = (el.height ?? 32);
+          const cx = (ax + gw / 2) * s, cy = (ay + gh / 2) * s;
+          const rx = (gw / 2) * s, ry = (gh / 2) * s;
+          const lw = Math.max(1, (el.lineWidth ?? 3));
+          // rx/ry are canvas pixels ((gw/2)*s) but lw is LED pixels, so
+          // insetting by lw/2 under-corrected by the scale factor while the
+          // stroke was drawn at lw*s -- the arc spilled outside the element's
+          // reported bounding box at any SCALE > 1, and the preview stopped
+          // matching the generated PIL output.
+          const lwPx = lw * s;
+          const startDeg = el.startAngle ?? 135;
+          const endDeg   = el.endAngle   ?? 45;
+          // Arc sweep: from startDeg clockwise to endDeg (PIL convention)
+          const totalSweep = ((endDeg - startDeg) + 360) % 360 || 360;
+          const pvGauge = opts.previewValues?.[el.binding?.key];
+          const pct = pvGauge !== undefined
+            ? Math.max(0, Math.min(100, parseFloat(pvGauge) || 0)) / 100
+            : Math.max(0, Math.min(100, el.previewPct ?? 65)) / 100;
+          const fillSweep = totalSweep * pct;
+          // A small imported gauge with a wide lineWidth would otherwise send
+          // a negative radius into ctx.ellipse(), which throws IndexSizeError
+          // and aborts the whole render() call.
+          const arx = Math.max(0, rx - lwPx / 2), ary = Math.max(0, ry - lwPx / 2);
+          // biome-ignore lint/correctness/useQwikValidLexicalScope: not Qwik -- this is an Alpine.js component, and the rule is about Qwik's $() serialization boundary, which does not exist here.
+          const toRad = deg => (deg - 90) * Math.PI / 180; // canvas 0=top, PIL 0=right → offset -90
+
+          // Track arc
+          if (el.hasTrack !== false) {
+            ctx.beginPath();
+            ctx.ellipse(cx, cy, arx, ary, 0, toRad(startDeg), toRad(startDeg + totalSweep), false);
+            ctx.strokeStyle = `rgb(${el.trackR ?? 40},${el.trackG ?? 40},${el.trackB ?? 40})`;
+            ctx.lineWidth = lwPx;
+            ctx.stroke();
+          }
+          // Fill arc
+          if (pct > 0) {
+            ctx.beginPath();
+            ctx.ellipse(cx, cy, arx, ary, 0, toRad(startDeg), toRad(startDeg + fillSweep), false);
+            ctx.strokeStyle = `rgb(${el.r},${el.g},${el.b})`;
+            ctx.lineWidth = lwPx;
+            ctx.stroke();
+          }
+          // Centre label
+          if (el.showLabel) {
+            const gfinfo = FONT_MAP[el.font || 'four_by_six'] || FONT_MAP.four_by_six;
+            const labelText = Math.round(pct * 100) + '%';
+            ctx.font = `${gfinfo.sizePx * s}px ${gfinfo.family}`;
+            ctx.fillStyle = `rgb(${el.labelR ?? 200},${el.labelG ?? 200},${el.labelB ?? 200})`;
+            const ltw = ctx.measureText(labelText).width;
+            ctx.fillText(labelText, cx - ltw / 2, cy + (gfinfo.sizePx * s) / 2);
+          }
+          break;
+        }
+
+        case 'marquee': {
+          const finfo = FONT_MAP[el.font] || FONT_MAP.press_start;
+          const text = el.text || 'Scrolling text';
+          const tw = text.length * finfo.charW * s;
+          const gap = (el.gap ?? 16) * s;
+          const totalW = tw + gap;
+          const tick = opts.animTick ?? 0;
+          const speed = (el.scrollSpeed ?? 1) * 2;
+          const scrolled = (tick * speed) % totalW;
+          // left: text enters from right; right: text enters from left
+          const startX = el.direction === 'right'
+            ? scrolled - tw
+            : matrixW * s - scrolled;
+          ctx.font = `${finfo.sizePx * s}px ${finfo.family}`;
+          ctx.fillStyle = `rgb(${el.r},${el.g},${el.b})`;
+          // Clip to canvas width so text doesn't bleed outside
+          ctx.save();
+          ctx.beginPath();
+          ctx.rect(0, ay * s - 1, matrixW * s, (finfo.sizePx + 2) * s);
+          ctx.clip();
+          for (let i = -1; i <= 2; i++) {
+            ctx.fillText(text, startX + i * totalW, (ay + finfo.sizePx) * s);
+          }
+          ctx.restore();
+          break;
+        }
+
+        case 'progress_bar': {
+          const bw = el.barWidth ?? 60, bh = el.barHeight ?? 6;
+          const pvPb = opts.previewValues?.[el.binding?.key];
+          const pct = pvPb !== undefined
+            ? Math.max(0, Math.min(100, parseFloat(pvPb) || 0)) / 100
+            : Math.max(0, Math.min(100, el.previewPct ?? 65)) / 100;
+          const rx = ax * s, ry = ay * s;
+          if (el.hasBg) {
+            ctx.fillStyle = `rgb(${el.bgR ?? 30},${el.bgG ?? 30},${el.bgB ?? 30})`;
+            ctx.fillRect(rx, ry, bw * s, bh * s);
+          }
+          const fillW = Math.max(0, Math.round(bw * pct));
+          if (fillW > 0) {
+            ctx.fillStyle = `rgb(${el.r},${el.g},${el.b})`;
+            ctx.fillRect(rx, ry, fillW * s, bh * s);
+          }
+          if (el.hasOutline) {
+            ctx.strokeStyle = `rgb(${el.outR ?? 100},${el.outG ?? 100},${el.outB ?? 100})`;
+            ctx.lineWidth = s;
+            ctx.strokeRect(rx, ry, bw * s, bh * s);
+          }
+          break;
+        }
+      }
+
+      if (belowBreakpoint) {
+        ctx.globalAlpha = 0.6;
+        const bb = getBoundingBox(el, matrixW, matrixH);
+        ctx.font = `${Math.max(8, s * 2)}px monospace`;
+        ctx.fillStyle = '#facc15';
+        ctx.fillText(`≥${el.minWidth}px`, bb.x * s, (bb.y + 4) * s);
+      }
+    } finally {
+      ctx.restore();
+    }
+  }
+
+  // ── Selection indicator ──────────────────────────────────────────────
+  function _drawSelection(ctx, el, SCALE, matrixW, matrixH) {
+    const bb = getBoundingBox(el, matrixW, matrixH);
+    const PAD = 2, s = SCALE;
+    const rx = bb.x * s - PAD, ry = bb.y * s - PAD;
+    const rw = bb.w * s + PAD * 2, rh = bb.h * s + PAD * 2;
+
+    ctx.save();
+    ctx.strokeStyle = '#3b82f6';
+    ctx.lineWidth = 1;
+    ctx.setLineDash([3, 2]);
+    ctx.strokeRect(rx, ry, rw, rh);
+    ctx.setLineDash([]);
+
+    if (el.xAnchor || el.yAnchor) {
+      ctx.font = `${Math.max(7, s)}px sans-serif`;
+      ctx.fillStyle = '#a78bfa';
+      const anchorText = [
+        el.xAnchor ? `x:${el.xAnchor[0]}` : '',
+        el.yAnchor ? `y:${el.yAnchor[0]}` : '',
+      ].filter(Boolean).join(' ');
+      if (anchorText) ctx.fillText(anchorText, rx + 1, ry - 2);
+    }
+
+    // Resize handles: on rect, rounded rect, ellipse
+    if (RESIZABLE_TYPES.includes(el.type)) {
+      const handles = _getRectHandles(el, matrixW, matrixH);
+      const HS = 5;
+      ctx.fillStyle = 'white';
+      ctx.strokeStyle = '#2563eb';
+      ctx.lineWidth = 1;
+      for (const pt of Object.values(handles)) {
+        const hx = pt.x * s - HS / 2;
+        const hy = pt.y * s - HS / 2;
+        ctx.fillRect(hx, hy, HS, HS);
+        ctx.strokeRect(hx, hy, HS, HS);
+      }
+    } else {
+      // Corner dots for non-rectangle elements
+      ctx.fillStyle = '#3b82f6';
+      const HS = 4;
+      for (const [hx, hy] of [
+        [rx - HS / 2, ry - HS / 2], [rx + rw - HS / 2, ry - HS / 2],
+        [rx - HS / 2, ry + rh - HS / 2], [rx + rw - HS / 2, ry + rh - HS / 2],
+      ]) ctx.fillRect(hx, hy, HS, HS);
+    }
+
+    ctx.restore();
+  }
+
+  // ── Dimension tooltip while dragging ─────────────────────────────────
+  function drawDragTooltip(ctx, el, SCALE, matrixW, matrixH) {
+    const bb = getBoundingBox(el, matrixW, matrixH);
+    const label = el.type === 'rectangle'
+      ? `${el.width}×${el.height}`
+      : `${bb.x},${bb.y}`;
+    const s = SCALE;
+    ctx.save();
+    ctx.font = `${Math.max(9, s * 1.5)}px monospace`;
+    const tw = ctx.measureText(label).width;
+    const tx = bb.x * s, ty = (bb.y - 2) * s;
+    ctx.fillStyle = 'rgba(0,0,0,0.7)';
+    ctx.fillRect(tx - 2, ty - 10, tw + 4, 12);
+    ctx.fillStyle = 'white';
+    ctx.fillText(label, tx, ty);
+    ctx.restore();
+  }
+
+  // ── Public API ───────────────────────────────────────────────────────
+
+  function init(canvasEl) {
+    _canvas = canvasEl;
+    _ctx = canvasEl.getContext('2d');
+  }
+
+  function setGrid(show) { _showGrid = show; }
+
+  function updateCanvasSize(matrixW, matrixH, SCALE) {
+    if (!_canvas) return;
+    _canvas.width = matrixW * SCALE;
+    _canvas.height = matrixH * SCALE;
+  }
+
+  function render(elements, selectedId, matrixW, matrixH, SCALE, opts = {}) {
+    if (!_ctx) return;
+    const cW = matrixW * SCALE, cH = matrixH * SCALE;
+
+    const bg = opts.bgColor;
+    _ctx.fillStyle = bg ? `rgb(${bg.r},${bg.g},${bg.b})` : '#000';
+    _ctx.fillRect(0, 0, cW, cH);
+
+    if (_showGrid) {
+      _ctx.strokeStyle = 'rgba(255,255,255,0.07)';
+      _ctx.lineWidth = 0.5;
+      for (let x = SCALE; x < cW; x += SCALE) {
+        _ctx.beginPath(); _ctx.moveTo(x, 0); _ctx.lineTo(x, cH); _ctx.stroke();
+      }
+      for (let y = SCALE; y < cH; y += SCALE) {
+        _ctx.beginPath(); _ctx.moveTo(0, y); _ctx.lineTo(cW, y); _ctx.stroke();
+      }
+    }
+
+    for (const el of elements) _drawElement(_ctx, el, SCALE, matrixW, matrixH, opts);
+
+    if (opts.showRuler) {
+      _ctx.save();
+      _ctx.fillStyle = 'rgba(255,255,255,0.08)';
+      _ctx.fillRect(0, 0, cW, SCALE);             // top strip
+      _ctx.fillRect(0, 0, SCALE, cH);             // left strip
+      _ctx.strokeStyle = 'rgba(255,255,255,0.5)';
+      _ctx.fillStyle = 'rgba(255,255,255,0.6)';
+      _ctx.font = `${Math.max(5, SCALE - 1)}px monospace`;
+      const step = SCALE >= 4 ? 8 : 16;
+      for (let px = 0; px <= matrixW; px += step) {
+        const cx = px * SCALE;
+        const major = px % 32 === 0;
+        _ctx.lineWidth = 0.5;
+        _ctx.beginPath(); _ctx.moveTo(cx, 0); _ctx.lineTo(cx, major ? SCALE : SCALE * 0.5); _ctx.stroke();
+        if (major && px > 0 && px < matrixW - 4) _ctx.fillText(String(px), cx + 1, SCALE - 1);
+      }
+      for (let py = 0; py <= matrixH; py += step) {
+        const cy = py * SCALE;
+        const major = py % 32 === 0;
+        _ctx.beginPath(); _ctx.moveTo(0, cy); _ctx.lineTo(major ? SCALE : SCALE * 0.5, cy); _ctx.stroke();
+        if (major && py > 0 && py < matrixH - 4) _ctx.fillText(String(py), 1, cy + SCALE - 1);
+      }
+      _ctx.restore();
+    }
+
+    if (opts.showGuides) {
+      _ctx.save();
+      _ctx.strokeStyle = 'rgba(255,60,60,0.45)';
+      _ctx.lineWidth = 1;
+      _ctx.setLineDash([4, 3]);
+      const mx = Math.floor(cW / 2) + 0.5;
+      const my = Math.floor(cH / 2) + 0.5;
+      _ctx.beginPath(); _ctx.moveTo(mx, 0); _ctx.lineTo(mx, cH); _ctx.stroke();
+      _ctx.beginPath(); _ctx.moveTo(0, my); _ctx.lineTo(cW, my); _ctx.stroke();
+      _ctx.setLineDash([]);
+      _ctx.restore();
+    }
+
+    const sel = selectedId != null ? elements.find(e => e.id === selectedId) : null;
+    if (sel) {
+      _drawSelection(_ctx, sel, SCALE, matrixW, matrixH);
+      if (opts.showTooltip) drawDragTooltip(_ctx, sel, SCALE, matrixW, matrixH);
+    }
+  }
+
+  return {
+    init, render, setGrid, updateCanvasSize,
+    hitTest, getBoundingBox, computeActualPos, resolveAnchor,
+    getResizeHandle, getCursorForHandle,
+    ELEMENT_DEFAULTS, FONT_MAP, DISPLAY_PRESETS, RESIZABLE_TYPES,
+  };
+})();

web_interface/templates/v3/composer/manager.py.j2

@@ -0,0 +1,302 @@
+"""
+{{ plugin_name }} — LEDMatrix Plugin
+Generated by LEDMatrix Plugin Composer on {{ generated_date }}
+
+Extension points:
+  update()              → add HTTP/MQTT data-fetching logic here
+  _get_display_values() → map fetched data to display strings
+  display()             → add new elements or adapt layout per display size
+"""
+from src.plugin_system.base_plugin import BasePlugin
+{% if has_clock or has_countdown %}
+from datetime import datetime
+{% endif %}
+{% if has_blink %}
+import time
+{% endif %}
+{% if has_text_template %}
+from collections import defaultdict
+{% endif %}
+
+
+class {{ class_name }}(BasePlugin):
+
+    def __init__(self, plugin_id, config, display_manager, cache_manager, plugin_manager):
+        super().__init__(plugin_id, config, display_manager, cache_manager, plugin_manager)
+{% for var in config_vars %}
+        self.{{ var.key }} = config.get({{ var.key | tojson }}, {{ var.default | tojson }})
+{% endfor %}
+        # Live data cache — populated by update(); always {} in static layouts
+        self._data = {}
+
+    def update(self):
+        """Fetch and refresh display data.
+
+        For dynamic plugins: fetch from APIs/MQTT here and store in self._data.
+        _get_display_values() will read self._data to produce display strings.
+        """
+        # --- Data sources (add fetch logic here for dynamic plugins) ---
+        pass
+
+    def _get_display_values(self):
+        """Map config variables and live data to display-ready strings.
+
+        This is the single extension point for v2 data sources:
+        add self._data lookups here once update() populates them.
+        """
+        return {
+{% for var in config_vars %}
+            {{ var.key | tojson }}: str(self.{{ var.key }}),
+{% endfor %}
+        }
+
+    def display(self, force_clear=False):
+        try:
+{% if has_text_template %}
+            values = defaultdict(str, self._get_display_values())
+{% else %}
+            values = self._get_display_values()
+{% endif %}
+
+            if force_clear:
+                self.display_manager.clear()
+
+            width = self.display_manager.width
+            height = self.display_manager.height
+{% if bg_color %}
+
+            self.display_manager.draw.rectangle([0, 0, width, height], fill={{ bg_color }})
+{% endif %}
+
+            # ── Elements (rendered bottom to top) ──────────────────────────
+{% for el in elements %}
+{% set p = "                " if el.min_width > 0 else "            " %}
+{% set pi = (p + "    ") if el.blink else p %}
+{% if el.min_width > 0 %}
+            if width >= {{ el.min_width }}:  # breakpoint: {{ el.min_width }}px+ displays only
+{% endif %}
+{% if el.blink %}
+{{ p }}if int(time.time() * 2) % 2:
+{% endif %}
+
+{% if el.type == 'text' %}
+{{ pi }}self.display_manager.draw_text(
+{% if el.text_is_template %}
+{{ pi }}    {{ el.text | tojson }}.format_map(values),
+{% else %}
+{{ pi }}    {{ el.text | tojson }},
+{% endif %}
+{{ pi }}    x={{ el.x_expr }}, y={{ el.y_expr }},
+{{ pi }}    color={{ el.rgb_tuple }},
+{{ pi }}    font=self.display_manager.{{ el.font_attr }},
+{{ pi }})
+{% if el.text2 %}
+{{ pi }}self.display_manager.draw_text(
+{% if el.text_is_template %}
+{{ pi }}    {{ el.text2 | tojson }}.format_map(values),
+{% else %}
+{{ pi }}    {{ el.text2 | tojson }},
+{% endif %}
+{{ pi }}    x={{ el.x2_expr }}, y={{ el.y2_expr }},
+{{ pi }}    color={{ el.rgb_tuple }},
+{{ pi }}    font=self.display_manager.{{ el.font_attr }},
+{{ pi }})
+{% endif %}
+{% elif el.type == 'dynamic_text' %}
+{% if el.binding_source == 'config' %}
+{{ pi }}self.display_manager.draw_text(
+{{ pi }}    values.get({{ el.binding_key | tojson }}, ''),
+{{ pi }}    x={{ el.x_expr }}, y={{ el.y_expr }},
+{{ pi }}    color={{ el.rgb_tuple }},
+{{ pi }}    font=self.display_manager.{{ el.font_attr }},
+{{ pi }})
+{% else %}
+{{ pi }}pass  # non-config dynamic_text binding draws nothing
+{% endif %}
+{% elif el.type == 'clock' %}
+{{ pi }}self.display_manager.draw_text(
+{{ pi }}    datetime.now().strftime({{ el.format | tojson }}),
+{{ pi }}    x={{ el.x_expr }}, y={{ el.y_expr }},
+{{ pi }}    color={{ el.rgb_tuple }},
+{{ pi }}    font=self.display_manager.{{ el.font_attr }},
+{{ pi }})
+{% if el.format2 %}
+{{ pi }}self.display_manager.draw_text(
+{{ pi }}    datetime.now().strftime({{ el.format2 | tojson }}),
+{{ pi }}    x={{ el.x2_expr }}, y={{ el.y2_expr }},
+{{ pi }}    color={{ el.rgb_tuple }},
+{{ pi }}    font=self.display_manager.{{ el.font_attr }},
+{{ pi }})
+{% endif %}
+{% elif el.type == 'countdown' %}
+{{ pi }}_cd_target = float(values.get({{ el.binding_key | tojson }}, 0) or 0)
+{{ pi }}_cd_secs = max(0.0, _cd_target - datetime.now().timestamp())
+{% if el.countdown_format == 'dhms' %}
+{{ pi }}_cd_d, _cd_rem = divmod(int(_cd_secs), 86400)
+{{ pi }}_cd_h, _cd_rem = divmod(_cd_rem, 3600)
+{{ pi }}_cd_m, _cd_s = divmod(_cd_rem, 60)
+{{ pi }}_cd_str = f'{_cd_d}d {_cd_h:02d}:{_cd_m:02d}:{_cd_s:02d}'
+{% elif el.countdown_format == 'hms' %}
+{{ pi }}_cd_h, _cd_rem = divmod(int(_cd_secs), 3600)
+{{ pi }}_cd_m, _cd_s = divmod(_cd_rem, 60)
+{{ pi }}_cd_str = f'{_cd_h}h {_cd_m:02d}:{_cd_s:02d}'
+{% elif el.countdown_format == 'dhm' %}
+{{ pi }}_cd_d, _cd_rem = divmod(int(_cd_secs), 86400)
+{{ pi }}_cd_h, _cd_m = divmod(_cd_rem // 60, 60)
+{{ pi }}_cd_str = f'{_cd_d}d {_cd_h:02d}h {_cd_m:02d}m'
+{% else %}
+{{ pi }}_cd_d, _cd_rem = divmod(int(_cd_secs), 86400)
+{{ pi }}_cd_h = _cd_rem // 3600
+{{ pi }}_cd_str = f'{_cd_d}d {_cd_h}h'
+{% endif %}
+{{ pi }}self.display_manager.draw_text(
+{{ pi }}    _cd_str,
+{{ pi }}    x={{ el.x_expr }}, y={{ el.y_expr }},
+{{ pi }}    color={{ el.rgb_tuple }},
+{{ pi }}    font=self.display_manager.{{ el.font_attr }},
+{{ pi }})
+{% elif el.type == 'rectangle' %}
+{{ pi }}self.display_manager.draw.rectangle(
+{{ pi }}    [{{ el.x_expr }}, {{ el.y_expr }}, {{ el.x2_expr }}, {{ el.y2_expr }}],
+{{ pi }}    fill={{ el.fill_tuple }},
+{{ pi }}    outline={{ el.outline_tuple }},
+{{ pi }})
+{% elif el.type == 'arc' %}
+{{ pi }}self.display_manager.draw.arc(
+{{ pi }}    [{{ el.x_expr }}, {{ el.y_expr }}, {{ el.x2_expr }}, {{ el.y2_expr }}],
+{{ pi }}    start={{ el.start_angle }}, end={{ el.end_angle }},
+{{ pi }}    fill={{ el.rgb_tuple }},
+{{ pi }}    width={{ el.line_width }},
+{{ pi }})
+{% elif el.type == 'ellipse' %}
+{{ pi }}self.display_manager.draw.ellipse(
+{{ pi }}    [{{ el.x_expr }}, {{ el.y_expr }}, {{ el.x2_expr }}, {{ el.y2_expr }}],
+{{ pi }}    fill={{ el.fill_tuple }},
+{{ pi }}    outline={{ el.outline_tuple }},
+{{ pi }})
+{% elif el.type == 'pixel' %}
+{{ pi }}self.display_manager.draw.point(
+{{ pi }}    [{{ el.x_expr }}, {{ el.y_expr }}],
+{{ pi }}    fill={{ el.rgb_tuple }},
+{{ pi }})
+{% elif el.type == 'rounded_rectangle' %}
+{{ pi }}self.display_manager.draw.rounded_rectangle(
+{{ pi }}    [{{ el.x_expr }}, {{ el.y_expr }}, {{ el.x2_expr }}, {{ el.y2_expr }}],
+{{ pi }}    radius={{ el.border_radius }},
+{{ pi }}    fill={{ el.fill_tuple }},
+{{ pi }}    outline={{ el.outline_tuple }},
+{{ pi }})
+{% elif el.type in ('line', 'divider') %}
+{{ pi }}self.display_manager.draw.line(
+{{ pi }}    [{{ el.x0_expr }}, {{ el.y0_expr }}, {{ el.x1_expr }}, {{ el.y1_expr }}],
+{{ pi }}    fill={{ el.rgb_tuple }},
+{{ pi }}    width={{ el.line_width }},
+{{ pi }})
+{% elif el.type == 'pips' %}
+{{ pi }}_pip_filled = max(0, min({{ el.pip_count }}, int(float(values.get({{ el.binding_key | tojson }}, 0) or 0))))
+{{ pi }}for _pip_i in range({{ el.pip_count }}):
+{{ pi }}    _pip_x = ({{ el.x_expr }}) + _pip_i * ({{ el.pip_size }} + {{ el.pip_spacing }})
+{{ pi }}    _pip_color = {{ el.fill_tuple }} if _pip_i < _pip_filled else {{ el.empty_tuple }}
+{% if not el.show_empty %}
+{{ pi }}    if _pip_i >= _pip_filled:
+{{ pi }}        continue
+{% endif %}
+{{ pi }}    self.display_manager.draw.rectangle(
+{{ pi }}        [_pip_x, {{ el.y_expr }}, _pip_x + {{ el.pip_size }} - 1, ({{ el.y_expr }}) + {{ el.pip_size }} - 1],
+{{ pi }}        fill=_pip_color,
+{{ pi }}    )
+{% elif el.type == 'sparkline' %}
+{{ pi }}_sl_raw = str(values.get({{ el.binding_key | tojson }}, '') or '')
+{{ pi }}_sl_vals = [float(v.strip()) for v in _sl_raw.split(',') if v.strip()][:{{ el.bar_count }}]
+{{ pi }}_sl_vals += [0.0] * max(0, {{ el.bar_count }} - len(_sl_vals))
+{{ pi }}_sl_max = max(_sl_vals) if any(_sl_vals) else 1.0
+{{ pi }}_sl_bw = max(1, ({{ el.bar_width_px }} - {{ el.bar_spacing }} * ({{ el.bar_count }} - 1)) // {{ el.bar_count }})
+{% if el.bg_tuple != 'None' %}
+{{ pi }}self.display_manager.draw.rectangle(
+{{ pi }}    [{{ el.x_expr }}, {{ el.y_expr }}, ({{ el.x_expr }}) + {{ el.bar_width_px }}, ({{ el.y_expr }}) + {{ el.bar_height_px }}],
+{{ pi }}    fill={{ el.bg_tuple }},
+{{ pi }})
+{% endif %}
+{{ pi }}for _sl_i, _sl_v in enumerate(_sl_vals):
+{{ pi }}    _sl_norm = max(0.0, min(1.0, _sl_v / (_sl_max or 1)))
+{{ pi }}    _sl_bh = max(1, round({{ el.bar_height_px }} * _sl_norm))
+{{ pi }}    _sl_bx = ({{ el.x_expr }}) + (_sl_bw + {{ el.bar_spacing }}) * _sl_i
+{{ pi }}    _sl_by = ({{ el.y_expr }}) + {{ el.bar_height_px }} - _sl_bh
+{{ pi }}    self.display_manager.draw.rectangle(
+{{ pi }}        [_sl_bx, _sl_by, _sl_bx + _sl_bw - 1, _sl_by + _sl_bh - 1],
+{{ pi }}        fill={{ el.fill_tuple }},
+{{ pi }}    )
+{% elif el.type == 'gauge' %}
+{{ pi }}_gv = max(0.0, min(100.0, float(values.get({{ el.binding_key | tojson }}, 0) or 0)))
+{{ pi }}_g_total = (({{ el.end_angle }} - {{ el.start_angle }}) % 360) or 360
+{{ pi }}_g_sweep = _g_total * _gv / 100.0
+{% if el.track_tuple != 'None' %}
+{{ pi }}self.display_manager.draw.arc(
+{{ pi }}    [{{ el.x_expr }}, {{ el.y_expr }}, {{ el.x2_expr }}, {{ el.y2_expr }}],
+{{ pi }}    start={{ el.start_angle }}, end={{ el.start_angle }} + _g_total,
+{{ pi }}    fill={{ el.track_tuple }},
+{{ pi }}    width={{ el.line_width }},
+{{ pi }})
+{% endif %}
+{{ pi }}if _g_sweep > 0:
+{{ pi }}    self.display_manager.draw.arc(
+{{ pi }}        [{{ el.x_expr }}, {{ el.y_expr }}, {{ el.x2_expr }}, {{ el.y2_expr }}],
+{{ pi }}        start={{ el.start_angle }}, end={{ el.start_angle }} + _g_sweep,
+{{ pi }}        fill={{ el.rgb_tuple }},
+{{ pi }}        width={{ el.line_width }},
+{{ pi }}    )
+{% if el.show_label %}
+{{ pi }}_g_cx = ({{ el.x_expr }}) + ({{ el.x2_expr }} - ({{ el.x_expr }})) // 2
+{{ pi }}_g_cy = ({{ el.y_expr }}) + ({{ el.y2_expr }} - ({{ el.y_expr }})) // 2
+{{ pi }}self.display_manager.draw_text(
+{{ pi }}    f'{int(_gv)}%',
+{{ pi }}    x=_g_cx, y=_g_cy,
+{{ pi }}    color={{ el.label_tuple }},
+{{ pi }}    font=self.display_manager.{{ el.font_attr }},
+{{ pi }})
+{% endif %}
+{% elif el.type == 'marquee' %}
+{{ pi }}_{{ el.data_key }}_text = {{ el.text | tojson }}
+{{ pi }}_{{ el.data_key }}_tw = len(_{{ el.data_key }}_text) * {{ el.char_w }}
+{{ pi }}_{{ el.data_key }}_x = int(self._data.get({{ el.data_key | tojson }}, width))
+{% if el.direction == 'right' %}
+{{ pi }}_{{ el.data_key }}_x += {{ el.scroll_speed }}
+{{ pi }}if _{{ el.data_key }}_x > width:
+{{ pi }}    _{{ el.data_key }}_x = -(_{{ el.data_key }}_tw + {{ el.gap }})
+{% else %}
+{{ pi }}_{{ el.data_key }}_x -= {{ el.scroll_speed }}
+{{ pi }}if _{{ el.data_key }}_x < -(_{{ el.data_key }}_tw + {{ el.gap }}):
+{{ pi }}    _{{ el.data_key }}_x = width
+{% endif %}
+{{ pi }}self._data[{{ el.data_key | tojson }}] = _{{ el.data_key }}_x
+{{ pi }}self.display_manager.draw_text(
+{{ pi }}    _{{ el.data_key }}_text,
+{{ pi }}    x=_{{ el.data_key }}_x, y={{ el.y_expr }},
+{{ pi }}    color={{ el.rgb_tuple }},
+{{ pi }}    font=self.display_manager.{{ el.font_attr }},
+{{ pi }})
+{% elif el.type == 'progress_bar' %}
+{{ pi }}_pb_x = {{ el.x_expr }}
+{{ pi }}_pb_y = {{ el.y_expr }}
+{{ pi }}_pb_pct = max(0.0, min(100.0, float(values.get({{ el.binding_key | tojson }}, 0) or 0))) / 100.0
+{{ pi }}_pb_fill_w = int({{ el.bar_width }} * _pb_pct)
+{{ pi }}self.display_manager.draw.rectangle(
+{{ pi }}    [_pb_x, _pb_y, _pb_x + {{ el.bar_width }}, _pb_y + {{ el.bar_height }}],
+{{ pi }}    fill={{ el.bg_tuple }},
+{{ pi }}    outline={{ el.outline_tuple }},
+{{ pi }})
+{{ pi }}if _pb_fill_w > 0:
+{{ pi }}    self.display_manager.draw.rectangle(
+{{ pi }}        [_pb_x, _pb_y, _pb_x + _pb_fill_w, _pb_y + {{ el.bar_height }}],
+{{ pi }}        fill={{ el.fill_tuple }},
+{{ pi }}    )
+{% else %}
+{{ pi }}pass  # element type "{{ el.type }}" draws nothing
+{% endif %}
+{% endfor %}
+            # ── End elements ───────────────────────────────────────────────
+
+            self.display_manager.update_display()
+
+        except Exception as e:
+            self.logger.error('Display error: %s', e, exc_info=True)