85 Commits

Include renames

Author	SHA1	Message	Date
ChuckandClaude Opus 5	92ac231138	fix(fonts): load 4x6 on its pixel grid, from any working directory (#565) ... * fix(fonts): load 4x6 on its pixel grid, from any working directory `extra_small_font` loaded 4x6-font.ttf at 6, off the face's 7px grid. Under `draw.fontmode = "1"` the mono rasteriser thresholds each glyph at 50% coverage, so every glyph lost its fourth column and deformed: christmas-countdown rendered "UNTIL" as "VM1JL". The advance is 5px at both sizes, so snapping to 7 reflows nothing. - Sizes in DisplayManager._load_fonts go through crisp_size() instead of literals. crisp_size / FONT_PIXEL_GRID / FONT_NAME_ALIASES move to src/common/font_layout.py; sports_card re-exports them. - Mirror the fix in VisualTestDisplayManager, the harness's fork of _load_fonts. Without it every golden is blessed at the old size. - Resolve bundled font paths against the install root, not the cwd. FontManager._resolve_asset_path now delegates to font_layout.resolve_asset_path (kept by name; plugins probe for it). - The startup banner's middle rung snaps to 7; the 5 rung stays off-grid on purpose (the only size that fits a dotted quad on 64px). - loading.py reads all plugin JSON as UTF-8 (cp1252 on Windows aborted check_plugin.py on a 0x9d byte). - check_plugin.py reports in ASCII and never dies on an unencodable char. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(fonts): resolve relative asset paths from the install root, not the cwd resolve_asset_path checked os.path.exists(relative_path) unconditionally, so a relative asset path was still resolved against the process cwd first -- exactly the dependency this module exists to remove. An unrelated working directory that happens to contain assets/fonts/4x6-font.ttf (a stale checkout, a copied assets folder, another project) would shadow the real bundled font instead of the install root ever being consulted. Only an absolute path is now returned as-is; a relative path always resolves against _INSTALL_ROOT first, matching the docstring's stated contract. FontManager._resolve_asset_path delegates to this function, so it's covered by the same fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-09-13 10:47:01 -04:00
ChuckandClaude Opus 5	bdb9a94033	refactor(api-v3): split the 10,469-line blueprint into a package (#553) ... * refactor(api-v3): split the 10,469-line blueprint into a package web_interface/blueprints/api_v3.py held 111 routes, 56 helpers and 181 functions in one module -- 9% of the core by line count and three times the next largest file. It becomes a package of nine route modules grouped by path segment, plus __init__.py for the shared imports, constants, Blueprint and helpers. Every route module decorates the SAME api_v3 Blueprint object, so endpoint names stay api_v3.<function>, the URL map is unchanged and app.py is untouched. Verified: 111 routes before, 111 after, byte-identical rules, endpoints and methods, and every endpoint still on the one blueprint. plugins 3,867 config 1,178 starlark 692 system 619 fonts 452 misc 398 wifi 361 display 326 backup 212 __init__ 1,787 (imports, constants, Blueprint, 56 helpers) Two things the URL-map check could not catch, both found by running the suite: 1. PROJECT_ROOT = Path(__file__).parent.parent.parent. Moving the code one directory deeper made that resolve to web_interface/ instead of the project root. Nothing failed at import; it surfaced as ~110 tests failing with 404s and "installation script not found", because every path built from it was one level too shallow. Now parents[3], and test_api_v3_url_map.py asserts PROJECT_ROOT/run.py exists so the next move cannot repeat it. 2. Module-attribute patching. Tests do monkeypatch.setattr(api_v3_module, "_BACKUP_EXPORT_DIR", ...) and a route module that binds such a name by value never sees the patch. The shared code therefore stays in __init__.py rather than moving to a _common submodule -- it has to live on the module the tests patch -- and the eleven names tests patch are read back through the package (_pkg.X) instead of bound by value. Those eleven were found by AST-scanning every setattr in the test tree, not by guessing; "time" is among them, used to drive a fake clock through the second-resolution credential-backup filenames. Test changes are confined to what genuinely moved: patch targets that now name the owning route module, imports of helpers, and six tests that scan the api_v3 source as a file and now read the package directory. Full suite: 4,278 passed, 68 skipped, 0 failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 * fix(api-v3): address CodeRabbit findings from the blueprint-split review Fixes to the api_v3 package split (PR #553), one per finding verified against the actual code: - __init__.py: _redact_credentials only blanked scalar values under a credential-named key; a bare list of secrets under such a key (e.g. tokens: ["a", "b"]) passed through untouched, since the list branch recursed with no memory that its key looked like a credential. Nested dicts still walk normally (a documented, tested behaviour -- a container like secrets: {api_key: ..., note: ...} is a section name, not a value to blank outright), but any value reached under a credential-shaped key is now actually blanked. - __init__.py: the OAuth helper script's raw stderr/stdout went to logger.error unredacted (CWE-532) right next to a comment claiming this was deliberate; the HTTP response already used the existing redact_text helper. Routed the log line through the same helper. - __init__.py / starlark.py: the standalone Starlark manifest fallback (used when the plugin instance isn't loaded) read-modified-wrote manifest.json with no lock, unlike StarlarkAppsPlugin._update_manifest_safe (plugin-repos/starlark-apps/manager.py), which already holds an flock for the same file when the plugin is loaded. Added _starlark_manifest_lock, mirroring that pattern, and wrapped every standalone read-modify-write call site in it. The app-config update route also wrote config.json and the manifest as two separate, non-transactional writes (a second, distinct finding at the same call site); config.json is now rolled back if the manifest write that follows it fails. - backup.py: restore options used bare bool() on values from the request, so {"restore_secrets": "false"} restored secrets anyway (bool("false") is True). Switched to the existing _coerce_to_bool helper already used for this exact purpose elsewhere in the package. - config.py: an automated import-rewrite mangled four user-facing validation strings and their neighbouring comments -- "Invalid start time" had become "Invalid start _pkg.time" (and likewise for "end time") in both the schedule and dim-schedule per-day validation paths. - display.py: `import _pkg.time as time_module` -- _pkg is a local alias for the package, not a real importable module, so this raised ModuleNotFoundError whenever a caller restarted an already-running display service via /display/on-demand/start, after the on-demand request was already written to cache. Fixed to `import time`. Audited the rest of the package for the same `_pkg.<module>` import mistake; every other `_pkg.` reference is a legitimate attribute read-through (`_pkg.time.time()`, `_pkg._get_starlark_plugin()`, ...), not a broken import statement. - fonts.py: validate_file_upload's max_size_mb parameter is silently unused by that helper (it only checks filename/extension) -- the font upload route saved arbitrarily large files as a result. Added the same seek-and-check pattern already used for the sibling .star upload. - wifi.py: two ad hoc, inconsistent bool coercions. POST /wifi/ap/auto-enable used bare bool(), so a JSON string "false" enabled it. POST /wifi/radio's enabled/force parsing recognized real bool and some strings but not int 1/0 (1 is True is False in Python). Factored one small _parse_bool_ish helper local to this file and used it at all three sites. Not changed: the "unknown/misspelled restore option keys default to True" half of the backup.py finding -- the file's own comment documents that a missing key deliberately means "restore everything," matching the already-existing JSON-parse-failure guard a few lines above it; only the bool-coercion defect was a real bug. Added or extended regression tests for every fix, following each area's existing test conventions. Full suite: 4328 passed, 62 skipped, 2 failed on both this branch and origin/main (missing tzdata package breaks two timezone-alias tests in test_onboarding_checklist.py, unrelated to this change) -- no new failures. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S3bPMESe2TfrGvbs1ef9c5 * fix(api-v3): reject unknown restore option keys CodeRabbit's review of the blueprint split (#553) asked that POST /backup/restore reject option keys outside RestoreOptions' known set. The follow-up commit fixed the bool("false")-is-True bug with _coerce_to_bool but never added the key check: a typo'd or renamed key (e.g. "restoreSecrets") is silently ignored by opts_dict.get(key, True), so the flag stays at its True default and secrets get restored despite the caller's request saying otherwise -- with no indication anything was wrong. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Vmcwf5vMgYqdt8bJTZtiwb * fix(api-v3): address CodeRabbit findings on the blueprint split - _redact_credentials: blank scalar descendants of objects reached through a credential-owned list (e.g. tokens: [{"value": "secret"}]) regardless of field name -- the existing name-based walk only protected direct dict values under a credential key, not list items. - wifi.py: reject enabled/force/auto_enable_ap_mode values _parse_bool_ish can't recognize (400) instead of silently treating them as False, which could disable Wi-Fi or the radio itself. - Starlark manifest locking: lock a stable manifest.json.lock sidecar instead of manifest.json itself, in both the standalone route path (_starlark_manifest_lock) and the plugin path (StarlarkAppsPlugin._save_manifest / _update_manifest_safe). manifest.json is replaced by an atomic rename on every write, which swaps in a fresh inode; a lock held on the old inode does not exclude a second locker that opens the path afresh right after the rename and gets the new inode, so two writers could race despite each holding "a lock". A sidecar that no write ever touches always resolves to the same inode for every locker. Skipped as stale: the "serialize the complete manifest read-modify-write" finding at api_v3/__init__.py -- every standalone handler that calls _write_starlark_manifest is already wrapped in _starlark_manifest_lock() on this branch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(api-v3): re-check reconciliation findings by the reconciler's own rules Both CodeRabbit findings on the merge commit, verified against the code first. Major, plugins.py: the stale-findings filter derived its own notion of "in config" and "on disk", and both were looser than the reconciliation module's. set(load_config()) also contains system keys, the secrets-file keys load_config() merges in, and non-dict values; and any directory holding a manifest.json counted as installed even when that manifest does not parse. Either looseness clears a finding that is still true -- and a secrets key read as a plugin is the precise bug the filter exists to stop reporting, so reintroducing that asymmetry while re-checking was the wrong way round. The two extractions now live in state_reconciliation.py as config_plugin_ids() and disk_plugin_ids(), with ignored_config_keys() and secrets_top_level_keys() alongside. _get_config_state() and _get_disk_state() use them too, so there is one definition rather than two that can drift. _get_disk_state() re-reads each manifest for version/name after taking membership from the shared extractor; that costs one extra small read per plugin on a path that runs once per boot. Minor, the new test: the fixture assigned api_v3.config_manager and api_v3.plugin_manager directly. Those live on a module-level blueprint singleton, so the mocks leaked into every later test that imports api_v3 -- pointing at a tmp_path already deleted. Both now go through monkeypatch.setattr, which restores them. This is the same pollution class that made an earlier test in this session break seven unrelated ones, so it is worth getting right. Five cases added for the parity itself: a secrets key, a system key and a non-dict value must not clear an "installed but missing from config" finding, and neither an unparseable manifest nor a .standalone-backup- directory may count as installed. All five fail against the looser version. Linux CI on the preceding commit: Core unit tests, plugin harness, CodeQL and CodeRabbit all pass. Codacy reads action_required on every commit of this branch including the first, so it is pre-existing and not from this work. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-11 10:07:38 -04:00
ChuckandClaude Opus 5	39f27d285d	fix(plugins): stop reconciliation inventing plugins and telling users to delete real config (#557) ... On a device running four installed, configured, working plugins, the overview banner read: Stale plugin config entries found: football-scoreboard, odds-ticker, data, ledmatrix-weather, starlark-apps. Remove them from config.json or reinstall via the Plugin Store. Every claim in that sentence was wrong, and following its advice would have deleted 4.9KB of working league settings. Four separate defects combined. 1. Secrets keys became phantom plugins. load_config() merges config_secrets.json into the config it returns, and the ignore list named only 'github' and 'youtube'. A 'data' key in that file therefore read as a plugin id and was reported as "in config but not on disk" forever. Read the secrets file's own top-level keys instead of hardcoding two of them. 2. The auto-fix clobbered real config. The handler for "on disk but not in config" assigned `config[plugin_id] = {'enabled': False}` unconditionally, so whenever detection was wrong it replaced a plugin's entire configuration with a stub. On the reported device it only failed to do so because the write hit EACCES. Now it refuses to overwrite an entry that already exists. 3. The banner gave backwards advice. plugin_missing_in_config ("on disk, not in config") and plugin_missing_on_disk ("in config, not on disk") are opposite problems, and both were rendered as "stale config entries ... remove them from config.json" -- which is correct for the second and destructive for the first. They are now reported separately, each with the advice that fits. 4. A stale verdict was served indefinitely. The result is a snapshot written once per run to a status file, and a run that fails to apply a fix also declares it will not retry. A condition that had since resolved kept being reported for hours. The status endpoint now re-checks stored findings against current state, dropping only what it can prove stale and keeping any kind it cannot re-verify. The secrets-key lookup is deliberately fail-safe: an unreadable, absent, malformed or non-path secrets location narrows the ignore set rather than raising. An earlier revision let TypeError escape, which the broad handler in _get_config_state() swallowed as "Error reading config state" -- emptying the config state and making every downstream detection wrong. The existing reconciliation tests caught it; there is now a regression test for it too. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-09-11 08:45:21 -04:00
ChuckandClaude Opus 5	577f5501a6	perf(plugins): stop re-deriving a display() signature the caller already cached (#549) ... display_controller resolves once, and caches, whether a plugin's display() takes a display_mode keyword -- self._plugin_accepts_display_mode, populated right before the dispatch. It then handed the executor a types.SimpleNamespace wrapping a closure, and execute_display() ran inspect.signature() on that to work out the same thing. Because the SimpleNamespace is rebuilt per call, the callable was new every time, so nothing inside the executor could ever cache it either. Measured at ~39us per dispatch on a Pi 4, for a value the caller had a line earlier. execute_display() now takes accepts_display_mode, falling back to inspecting only when a caller does not pass it, so existing callers are unaffected. Also documents two things that read as bugs and are not: - execute_with_timeout()'s timeout is advisory. Nothing cancels the thread -- Python cannot -- so on expiry the operation runs to completion in the background and only the caller gives up. A permanently hung plugin leaks a daemon thread per attempt. This is why callers holding a lock across the call must release it from inside the wrapped callable, as run()'s _release_display_lock already does. - Only the first display() of each mode goes through the executor; the per-frame loops call display() directly. That is deliberate: a thread per frame would cost more than an advisory timeout buys. Both loops now say so, so the asymmetry does not read as an oversight. Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-11 08:42:49 -04:00
ChuckandClaude Sonnet 5	fb3b293ace	fix(plugins): let a plugin ask to be polled faster while it has live content (#555) ... * fix(plugins): let a plugin ask to be polled faster while it has live content Reported: "the football plugin with live games only updates the live game in progress if I restart the display." The data path was never the problem. NFLLiveManager fetches ESPN with no cache, SportsLive.update() refreshes current_game in place when the game IDs are unchanged, and the scorebug redraws from the game dict every frame -- which is why the reporter's logs look healthy. The problem is cadence. _get_plugin_update_interval() read only the manifest's static update_interval, football's manifest pins that to 60, and the plugin's own live_update_interval (15s) was invisible to the scheduler. Measured on a rig during the fourth quarter of the game in the report: 23:21:49 23:22:50 23:23:50 23:24:50 23:25:50 <- exactly 60s apart A clock and score up to a minute stale during a two-minute drill reads as a frozen panel, and a restart is the one moment it is ever current. A single static number cannot say "every 15 seconds while a game is on, every 15 minutes in July", and only the plugin knows which is true. get_update_interval() lets it say so per tick; returning None means "no opinion" and the existing manifest/config resolution applies, so every plugin that predates this is unaffected. Requests are clamped to MIN_DYNAMIC_UPDATE_INTERVAL (5s): a plugin returning 0 would otherwise be re-entered on every tick of the render loop, busy-waiting against its own API. A hook that raises or returns a non-number is ignored rather than propagated -- a scheduler that fails on one plugin's bug stops updating all the others. Deliberately NOT changed: the manifest still beats config in the static path. That looked like the obvious fix -- user config being silently ignored -- until checking a real rig, where football and baseball both carry update_interval 3600 in config against a manifest 60, and weather 1800 against 60. Those values are stale precisely because nothing has been honouring them; making config win would have slowed three plugins by 60x, turning a one-minute lag into an hour. The dynamic hook makes the flip unnecessary. There is a test pinning the current precedence with that reasoning attached. Full suite: 4,283 passed, 68 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 * test(plugins): drive the real scheduler, not just the interval resolver test_plugin_dynamic_update_interval.py asserts that _get_plugin_update_interval() returns the number the plugin asked for. That is not the same claim as "the plugin gets updated more often", and the gap between those two is exactly where the original bug lived: the plugin knew it wanted 15s, said so in live_update_interval, and nothing downstream acted on it. So this ticks the real run_scheduled_updates() through a simulated hour and counts dispatches. Against pre-fix core it reports "10 updates in 10 minutes of a live game" -- the 60s manifest cadence, matching what was measured on a rig during the reported game. Against the fix it reports ~40. Also pins the regression that would be worse than the bug: an idle hour must still be ~60 updates, not 240. Asking for the live interval year-round would poll ESPN four times a minute all summer. Scope note, since it is easy to over-read this fix: the *switch* display path already refreshed the manager immediately before drawing, via _try_manager_display() -> _ensure_manager_updated(), which honours the manager's own 15s interval. So a switch-mode card was already <=15s stale at draw time before this change. What this fixes is the background cadence, which is what live-priority detection, Vegas content and scroll preparation all read. Full suite: 4,288 passed, 68 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 * fix(plugins): reject bool and -inf hook results in dynamic interval get_update_interval() ran bool through float() (bool is an int subclass, so True/False became 1.0/0.0) and only checked for +inf, not -inf. Both cases landed on the MIN_DYNAMIC_UPDATE_INTERVAL floor by coincidence instead of falling back to the static/manifest interval as invalid input should. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Tst9cied2ri9bH4QRWa6H --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-11 08:41:58 -04:00
ChuckandClaude Opus 5	968b953a51	fix(display): pin one text layout engine, and give the 5x7 BDF face a size (#539) ... * fix(display): pin one text layout engine, and give the 5x7 face a size Two ways a font could render differently on two machines running the same code, both found while diagnosing four plugins whose golden images passed on the machine that generated them and failed everywhere else. **Layout engine.** `ImageFont.truetype` picks its engine at load time: Raqm where the host Pillow was built with libraqm, Basic otherwise. The two round fractional glyph advances differently. `PressStart2P-Regular.ttf` at 8px has whole-pixel advances, so they agree — which is why most of the fleet matched everywhere and hid this. `4x6-font.ttf` at 6px does not: glyph positions drift cumulatively along a run, and the four plugins that draw body text in it (geochron, of-the-day, christmas-countdown, ledmatrix-weather's almanac) are exactly the four whose goldens travelled badly. Every core font load now goes through `src/common/font_layout.load_truetype`, which pins the Basic engine, so a render depends on the font file and the size and nothing else. Basic gives up complex-script shaping and kerning pairs; neither applies to bitmap-grid faces on an LED panel. Output is unchanged on a host without libraqm. **Zero font height.** `DisplayManager` built the 5x7 BDF face with `freetype.Face(path)` and never called `set_char_size`, so `face.size.height` stayed 0 and `get_font_height()` returned 0 for it — callers stacking rows by `prev_y + prev_height + gap` drew two lines on top of each other. The start-up line `Calendar font size: 0 pixels` has been printing the symptom all along. `font_manager._load_bdf_font` already called `set_char_size`, so whether measurement worked depended on which path loaded the face. `DisplayManager` now sets it too, and `get_font_height()` falls back to the strike the file declares rather than returning a zero line height. Fixes ChuckBuilds/ledmatrix-plugins#397 Refs ChuckBuilds/ledmatrix-plugins#371, #375, #378, #391 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(display): give the startup banner a rung that fits a full address at 64px CI caught what pinning the layout engine exposed rather than caused. `_fitting_font` walks PressStart2P then 4x6 at 6px, and "255.255.255.255" -- the widest thing the startup banner ever shows -- measures 66px at 4x6/6px against the 62 a 64x32 panel has to give. It used to squeak in only because the measurement depended on which layout engine the host Pillow happened to have; with the engine pinned it does not, so the rung the worst case actually needs is now in the ladder instead of implied: 4x6 at 5px, which measures 51. The fallback was wrong in the same place. When nothing in the ladder fit, it returned `self.font` -- the *widest* option, and precisely how "Initializing" came to run off the side of a 64px panel to begin with. It returns the narrowest face that loaded now. test/test_initializing_screen.py: 34 passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(display): name the exceptions the BDF strike read can raise Codacy flagged the try/except/pass. It was already narrow in intent -- a malformed strike table on the measurement path must degrade to "size unknown" rather than take the display down -- but a bare `except Exception: pass` says neither of those things and hides a genuinely broken font behind a silent 8px fallback. It now catches what reading `available_sizes` can actually raise and logs which face failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * chore: drop logo PNGs the render harness downloaded into the worktree These are fetched at runtime by the logo cache; they are not source, and they rode in on a `git add -A` while I was running check_plugin.py against this branch. Nothing in the change needs them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-09-08 09:40:00 -04:00
ChuckandClaude Opus 5	c9289e3a1d	fix(store): update_plugin silently did nothing for four installed plugins (#536) ... install_plugin() deliberately renames a plugin's directory to the MANIFEST id when it differs from the REGISTRY id, so registry `stocks` lands in `ledmatrix-stocks/`. Every lookup in _find_plugin_path() is by directory name, so update_plugin("stocks") found nothing, logged "Plugin not installed", and returned False. Nothing surfaced that to the user. Clicking update in the web UI was a no-op with no error, and the plugin stayed on a stale version indefinitely. Four installed plugins hit this on a real device -- leaderboard, music, stocks and weather -- found because a scripted update of eleven plugins failed on exactly those four. Adds a manifest-id scan as the LAST step of the resolution chain, so the two documented lookups above it (configured dir, then the sibling plugins/ fallback) keep their exact meaning and ordering. That ordering is pinned by test_discovery_path_contract.py, which characterises the divergence between the three resolvers on purpose; this extends the chain rather than reordering it. Directories renamed aside with '.standalone-backup-' during an install or rollback are skipped, since matching one would report a half-finished install as a live plugin. Also adds scripts/audit_render_path.py, which walks the call graph from display() and reports blocking calls reachable from it. display() runs on the render thread, so anything slow there stalls the panel; on a vsync-paced loop a single 15ms call drops a frame and a network round trip freezes the marquee. Two instances were already found the slow way, by reading frame-time histograms -- odds-ticker reading the scoreboard cache per frame, and soccer-scoreboard timing out inside update(). The audit finds that shape in the source instead. It is a heuristic and says so: a hit behind an interval check may be fine. It currently flags 23 calls across six plugins. The clearest is ledmatrix-music, whose display() falls back to an inline requests.get(timeout=5) when album art has not been prefetched -- a deliberate "show the art rather than go blank" tradeoff by its author, but up to five seconds of frozen panel. Reported, not changed; that is its owner's call. 185 store tests pass. Three of the six new tests fail without the fix. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-09-07 15:55:46 -04:00
ChuckandClaude Opus 5	d12323e7f1	perf(scroll): pace frames to the panel — 44→100 fps, stalls 14% → 0.02% (#523) ... * perf(scroll): pace frames to the panel, not to a fixed sleep Scrolling ran at 44-46 fps on a 2x128x64 chain and 14-17% of frames took 41-53ms, which reads as judder. Four independent causes, each measured on the hardware; details and the diagnostic recipe are in docs/SCROLL_PERFORMANCE.md. The high-FPS loop slept a flat 8ms after every render. display() has already blocked on the panel's vsync by then, so that sleep was added to a wait that had happened: ~4ms of render plus 8ms put each iteration at ~12ms against a 10ms refresh grid, so every swap missed a refresh and the loop settled at 50fps while asking for 125 -- with no headroom, so a further 14% of frames slipped again. It now sleeps only the remainder, with a 1ms floor so plugin threads still get the GIL. ScrollHelper stepped position on a wall clock at 1/scroll_delay steps per second. Plugins set scroll_delay to the frame period, so that comparison sat exactly on its own threshold: a frame arriving a hair early moved zero pixels and rendered an identical frame, dirty-tracking skipped the swap, it returned in ~2ms, and the beat repeated. No scroll_delay value tunes that out -- a shorter delay trades stalled frames for periodic double-steps. Both modes now accumulate elapsed time at the same configured speed, so position stays proportional to real time. Sub-pixel blending goes back to off by default. It renders a half-step by mixing two adjacent columns, which on a coarse panel showing pixel-font text alternates crisp and smeared frames and reads as shimmer -- visibly worse than integer stepping on the hardware. Vegas mode still opts in. disk_cache uses orjson when importable, falling back to the stdlib. Encoding a ~1MB record drops from 14.8ms to 5.4ms end-to-end, and that work holds the GIL while a marquee is on screen. display_manager also checksummed the whole framebuffer twice per frame (dirty tracking, then the preview snapshot); the snapshot now takes the checksum the caller already computed. New src/common/scroll_config.py resolves scroll settings in one place. Five ticker plugins each hand-rolled this and disagreed: odds-ticker ranked the deprecated scroll_pixels_per_second above the documented scroll_speed/delay pair, and because that key carries a schema default the documented settings were dead for every user (ChuckBuilds/ledmatrix-plugins#408), while ledmatrix-leaderboard read the same key only as a fallback. The resolver also warns when a speed will not advance a whole number of pixels per refresh, which is the property that actually determines whether a scroll looks smooth. scripts/build_rgbmatrix_nogil.sh rebuilds the rgbmatrix binding so it releases the GIL. Upstream declares SwapOnVSync without nogil, unlike SetPixel/Clear/Fill beside it, so the render thread held the GIL for the whole vsync wait and starved background threads into long uninterruptible bursts. The script patches, builds and self-verifies into a scratch tree; --install backs up the original and rolls back if the service does not come back healthy. Measured after: 100 fps locked, no stalls observed, render thread down from 51% to 19% of one core. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(display): keep the panel swap locked to vsync while scrolling Dirty tracking skipped SwapOnVSync for byte-identical frames. That is the right call for static content, but SwapOnVSync is also what paces the render loop, so skipping it skips the wait for the panel: a duplicate frame returns in ~8ms instead of ~10ms on a 100Hz panel, advances the strip only 0.8px instead of 1.0px, and so makes the next frame more likely to repeat as well. The effect sustains itself once it starts. Measured over 20 minutes on a 2x128x64 chain, both scrollers configured identically at 100 px/s: leaderboard 10ms x35, 11ms x3 (clean) odds-ticker 10ms x26, 8ms x7, 15ms x5 (~20% duplicates mid-scroll) The duplicates were not end-of-cycle idling -- 38% of fast frames fell within 90s of a scroll completion against 35% of normal frames, a null result. The trigger is per-frame work: odds does more of it, and more variably, so it is first to land a frame that advances less than a whole pixel. Pushing an identical frame costs one canvas copy. Falling out of vsync lock costs smooth motion. Static content is untouched, because is_currently_scrolling() expires on its own inactivity threshold -- covered by test_stale_scrolling_state_stops_forcing_pushes so a plugin that stops scrolling without saying so cannot pin the panel into always-push. Also de-flakes test_snapshot_still_written_on_skip, which asserted a strict mtime increase between two writes that can land in the same filesystem tick; it failed about two runs in three on Windows regardless of the code under test. The file is now backdated before the check. 156 tests pass on the Pi. Not yet confirmed by eye on the panel. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(scroll): report the frame-time tail, and stop the row-major blit Two problems, both found by looking at the panel rather than the metric. The frame-stats line reported ONE instantaneous frame every 5 seconds -- about 1 frame in 500 -- printed beside a 100-frame average. Both hide exactly the fault they are used to chase: a 2ms duplicate and a 21ms double-wait average to precisely 10ms, so a ticker stalling on half its frames still reports a healthy "Avg FPS: 100.0". That reading cost several rounds of chasing the wrong layer. The line now aggregates every frame since the last log and reports median, p95, max, min, and explicit stall and skip rates (past 1.5x the median missed a refresh; under half never reached the panel, because dirty tracking skipped the swap so the frame never waited on vsync). On the hardware this now reads: leaderboard 100.0 fps over 501 frames | median 10.00ms p95 10.05ms max 10.34ms | stalls 0 (0.0%) skips 0 (0.0%) The binding rebuild's blit patch becomes opt-in (RGB_PATCH_BLIT=1, default off). Reordering that loop to row-major changes what a torn frame looks like: column-major tearing shows as a vertical seam, row-major 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, which is what was reported on hardware and what went away when the blit was reverted. All of the measured gain comes from the SwapOnVSync change, so the risky half is simply not worth taking; the header says so. Also fixes --install resolving its paths against $HOME, which is /root under sudo, so it looked in /root/rgbmatrix-nogil-build and died with "no built module found" on a machine where the build had just succeeded. It now resolves SUDO_USER's home. Both build paths are verified on the Pi: default yields one GIL-release site, RGB_PATCH_BLIT=1 yields two. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * feat(scroll): let users pick a crisp speed for their own panel Whole-pixel motion was previously only available at multiples of the refresh rate -- 100, 200, 300 px/s on a 100Hz panel. 100 px/s crosses a 256px panel in 2.6s, which is brisk for reading, and everything slower had to blend (blur) or repeat frames unevenly (judder). There was no way to ask for 50 px/s and get clean motion. SwapOnVSync takes a framerate_fraction the display manager never passed. It holds each frame for N panel refreshes; the panel keeps refreshing at its full rate throughout, so holding costs nothing in flicker and only changes how often a NEW image is presented. That turns 50 px/s into one whole pixel every second refresh instead of half a pixel every refresh. The crisp speeds are therefore refresh_hz / hold * pixels_per_frame, and that ladder depends on the panel: a Pi Zero on a long chain has a different set of good speeds from a Pi 4 on a short one. crisp_ladder() enumerates them and solve_crisp() picks the best match for a requested speed. solve_crisp weights motion quality rather than picking the numerically nearest entry, which matters more than it sounds. Asked for 30 px/s, nearest-by-value answers 28.6 -- 2px jumps at 14fps -- over 33.3, which is single-pixel motion at 33fps and obviously better on the panel. The target is also clamped into the ladder's range first, because relative error saturates near 1.0 for a target far outside it and the quality penalty would otherwise answer "10000 px/s" with the slowest entry. configure() snaps to the ladder and applies the hold when given a display manager. Without one the hold silently cannot happen and motion falls back to fractional pixels, so it warns rather than failing quietly. set_frame_hold() resets to 1 when scrolling stops, so one plugin's pacing cannot leak into whatever is on screen next. scripts/scroll_speeds.py is the user-facing part: it prints the ladder for the configured rate, measures what the panel ACTUALLY manages (--measure, for hardware that cannot reach its configured limit), highlights the nearest option to a wanted speed, and demos one live. It never starts or stops the display service itself -- doing that inside a script stranded the panel twice today. Speeds below ~20 px/s remain stepped regardless. That is the pixel pitch, not a software limit. Also fixes the dirty-tracking test spy, which stubbed SwapOnVSync with a single-argument function and would have masked the new call as a failed push, and rewrites a configure() test that had started passing for the wrong reason: it asserted a judder warning, which snapping now prevents, and was matching the unrelated "hold could not be applied" warning instead. 183 tests pass on the Pi. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(scroll): tie the frame hold to the scroll, not the plugin The hold applied in configure() never reached the panel. Plugins share one display manager, and set_scrolling_state(False) -- fired whenever ANY other plugin finishes its scroll -- reset the hold to 1. A hold set once at plugin construction was therefore always gone by the time that plugin rendered. The symptom was a log line that lied. ledmatrix-stocks reported Scroll configured: 50.0 px/s (1px every 2 refreshes = 50.0 fps, smooth) while the panel measured 100.0 fps, median 10.00ms. Config, resolution and snapping were all correct; only the pacing silently was not applied. set_scrolling_state(is_scrolling, frame_hold=1) now carries it, so the hold lives exactly as long as the scroll that asked for it. configure() reports the value as ScrollSettings.frame_hold instead of applying it -- applying it behind the caller's back could never have been right on a shared display manager. Existing callers are unaffected; the default keeps one frame per refresh. Verified on hardware: stocks at 50 px/s now measures 50.0 fps over 251 frames | median 20.00ms p95 20.09ms | stalls 0 skips 0 20.00ms being exactly two refreshes, with the panel still refreshing at 100Hz underneath so flicker is unchanged. test_another_plugin_stopping_does_not_strand_a_hold pins the interaction that broke this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(scroll,cache): resolve CodeRabbit review on #523 Eight findings, all reproduced before fixing. scroll_config.configure() read the refresh rate *after* resolve() had already used it. resolve() fills in target_fps, pixels_per_frame and the judder warning from that rate, so on a 60Hz panel every one of them described 100Hz -- and with snap_to_crisp=False nothing downstream corrected it, so set_target_fps() paced the helper to 100 FPS. The rate is now settled first, and falls back to the global config rather than straight to the default. refresh_hz_from_config() used `(cfg.get("display") or {}).get(...)`, which raises AttributeError when either level is truthy but not a mapping -- out of a function whose whole contract is a rate or a default. The frame-stats line reported the upper-middle sample as the median and the 96th sorted sample as p95 of 100. Both are also thresholds (stalls at 1.5x the median, skips at 0.5x), so the counts were biased too. The arithmetic is now in frame_stats()/format_frame_stats(), testable without a clock. configure()'s docstring and docs/SCROLL_PERFORMANCE.md still said it applies the frame hold and warns when it cannot. It deliberately does neither since "tie the frame hold to the scroll, not the plugin"; a caller following the old text would omit set_scrolling_state() and slow snapped speeds would still present every refresh. disk_cache had no policy for non-finite floats: orjson writes null, the stdlib writes NaN/Infinity, and orjson then rejects those legacy files so DiskCache.get deleted them as corrupt. One behaviour on both paths now -- write null, keep legacy records readable. allow_nan=False detects the values; the replacement walk runs only when there is one, so the ordinary write path is byte-identical and pays nothing. build_rgbmatrix_nogil.sh picked the build artifact with a glob piped to `head -1`, which sorts cpython-311 ahead of cpython-313, so a stale .so staged in from the source tree was installed as core.so while the GIL check -- which reads the generated core.cpp, not the .so -- still passed. It now requires the current interpreter's exact ABI name and fails closed. Its systemctl calls were also unchecked under `set -uo pipefail`: a failed stop left the old service running, the following start succeeded as a no-op, and the health check reported SUCCESS for a binding that was never loaded. orjson floor raised to 3.11.6 for CVE-2025-67221 (unbounded recursion in dumps); it covers the project's Python 3.10-3.13 range. Adds test/test_cache_nonfinite_floats.py (14) plus regression tests in test_scroll_config.py and test_scroll_helper.py. 9 of the cache tests and 9 of the scroll_config tests fail against the pre-fix code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 * test(harness): keep the visual double's signature tied to production Moves set_scrolling_state's frame_hold into the test double here, where DisplayManager gains it, rather than in #534 where it arrived a PR early. CodeRabbit flagged the #534 version correctly: a double that accepts an argument production does not lets the call pass every harness run and raise TypeError on the panel, which is the one failure a safety harness exists to prevent. The drift has now gone both ways across two branches -- double behind production on this branch, double ahead of it on #534 -- so it is pinned instead of remembered. test_display_double_parity.py compares the two signatures and fails with the direction of the drift named. It reads the files with ast rather than importing them, because display_manager imports rgbmatrix at module scope and this check should hold on a laptop and in CI as well as on a Pi. Plugins begin passing frame_hold in ledmatrix-plugins#462, which is why production and the double both need it before that lands. Full suite: 3889 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-09-07 13:37:54 -04:00
ChuckandClaude Opus 5	a0d3e64099	fix: ten defects found validating the whole plugin fleet on hardware (#534) ... * fix(core): register tom_thumb, accept frame_hold in the test double, wire api_v3's managers Three independent fixes found while validating every plugin on a 256x64 rig. FontManager never registered tom_thumb even though assets/fonts/tom-thumb.bdf ships with the core, so every plugin offering it logged "Font family 'tom_thumb' not found" (16 warnings per countdown render) and had to carry a private loader to use a bundled font. Closes #524. VisualTestDisplayManager.set_scrolling_state() lacked the frame_hold parameter that DisplayManager gained, so any plugin passing it died with TypeError at render time and failed every size. Nine plugins now make that call; ledmatrix-stocks and ledmatrix-leaderboard were failing outright and the other seven only passed because their scroll path was unreachable without data. Closes #525. api_v3 declared module-level config_manager/plugin_manager = None that nothing ever assigned -- app.py sets the blueprint attributes, which the other 150+ call sites use. Three sites read the decoys, so /health reported the config unreadable and the plugin system uninitialised (making "degraded" permanent and unreachable-by-design) and /display/current fell back to a hardcoded 128x64 on every rig. The decoys are removed rather than assigned, so a bare name is now a NameError at test time instead of a silent None. The same function's first-call uptime was computed from two separate clock reads and came out negative. Closes #529. Verified on the rig: both previously-failing plugins render, the tom_thumb warnings are gone, /health reports "healthy" with all three checks passing, and /display/current reports the real 256x64. * fix(core): unique snapshot temp name, honour on-demand requests, skip empty starlark The preview snapshot wrote through a fixed "<snapshot>.tmp". /tmp is world-writable and sticky, and the display service runs as a different user from the tooling, so a leftover temp owned by anyone else became unopenable even by root -- fs.protected_regular refuses O_CREAT on a foreign file in a sticky directory. The preview and the health check's liveness proxy then froze until someone deleted the file by hand; on the test rig that meant 23 hours of a healthy display reporting "hardware: stale". Now uses tempfile.mkstemp with cleanup on failure, matching the hardware-status write a few hundred lines above. Closes #528. _poll_on_demand_requests read its mailbox with max_age=3600, and get() defaults the in-memory TTL to max_age -- so the first request was pinned in memory for an hour and every later poll returned that stale copy. No second on-demand request was honoured until the service restarted, while the API kept returning 200. get() already documents memory_ttl=0 for exactly this cross-process case. The consumed request is also now deleted: leaving it on disk meant a restart replayed the previous request, activated it, and ignored the one the caller had just made. Closes #530. starlark-apps returned None from display() when it has no app to show, which is the state of every install without Pixlet and of a fresh one before any app is added. The controller only skips on a boolean False, so that held a black panel for the full display_duration instead of rotating on. Closes #456 (core side). Verified on the rig: two consecutive on-demand requests with no restart between them are both activated, where the second was previously dropped in silence. * perf(harness): share one cache across a plugin's renders _instantiate built a fresh MockCacheManager for every (size, mode), and that mock is a per-instance in-memory dict, so each render was a cold start. A plugin that fetches per game or per player re-fetched everything N times over -- baseball-scoreboard at one size took 840s for nine renders where the arithmetic said ~72s, and at eight sizes it exceeded a 900s timeout. The second and later renders also never exercised the cache-hit path, which is what a running rig executes almost all of the time, so a caching regression could not be caught here. The cache is now built once per render_plugin_matrix call and threaded down. The display manager stays per-render -- the bounds checking depends on that -- so only fetched data is shared. Measured on the rig, same render counts and same goldens: tide-display 2s -> 1s (32 renders) cricket-scoreboard 10s -> 3s (24 renders) No pass/fail change across tide-display, cricket-scoreboard, clock-simple, geochron, christmas-countdown, of-the-day, web-ui-info and incoming-packages. Closes #533. * fix(scripts): run standalone plugin tests instead of collecting nothing run_plugin_tests.py discovered every plugin test file and handed the lot to pytest. Most plugin tests are standalone scripts -- module-level main() plus an `if __name__ == "__main__"` guard, signalling through an exit code -- and pytest collects zero items from those. The run printed how many files it had *found*, then "no tests ran", and exited without executing any of them. On a rig with all 44 first-party plugins that is 151 of 248 files. Files are now classified and each kind runs under the right runner: pytest for real test modules, subprocess for scripts, honouring the 0 pass / 2 skip / 1 fail convention ledmatrix-plugins' own runner established (a script that wants a tty or an LED matrix is a skip, not a regression). Before: $ python3 scripts/run_plugin_tests.py -p countdown -d ~/LEDMatrix/plugin-repos Found 1 test file(s) collected 0 items no tests ran in 0.31s rc=0 After: Found 1 test file(s) -- 0 collectable, 1 standalone script(s) 1 passed, 0 skipped, 0 failed (scripts) rc=0 Verified across three shapes: countdown (1 script), jellyfin-now-playing and pomodoro-timer (pytest only, 16 and 42 tests), and ledmatrix-flights (11 files split 4 collectable / 7 scripts, all seven of which had never run). Closes #532. Running the flights scripts for the first time also surfaced four genuinely failing tests there, hidden by the mirror-image bug in the plugins repo's own runner -- filed as ChuckBuilds/ledmatrix-plugins#464 and #465. * fix(harness): give an empty-looking mode a few frames before warning about it check_plugin's "drew nothing but display() returned X" warning fired on a single frame, rendered with force_clear=True, under a frozen clock. All three defeat a scrolling plugin, whose first frame is legitimately its blank scroll-in buffer. Across 44 first-party plugins, 60 of 76 warnings were false -- the rate at which people stop reading a warning, which matters because the true positives are real: a mode that draws nothing and does not return False holds a blank panel for its whole display duration. An apparently-empty frame is now re-driven for up to 48 more frames with force_clear=False (force_clear means "reset the scroll", so repeating it would redraw frame 1 for ever) and with the clock advancing -- freezegun's factory where time is frozen, a real sleep where it is not, since scroll position is usually a function of elapsed time. The first frame that draws content replaces the result. The clock is moved back afterwards. It is shared by every render in the matrix, so time borrowed by the probe leaked into later modes and drifted their goldens -- f1_upcoming picked up 5 spurious drifts before this was restored. Measured on the rig: empty warns check before after f1-scoreboard 42 0 48 PASS / 0 FAIL, goldens intact ledmatrix-elections 16 0 16 PASS / 0 FAIL on-air 8 8 true positive, kept nfl-draft 8 8 true positive, kept clock-simple/geochron/ 0 0 unchanged christmas-countdown 58 false positives gone, both true positives kept, no golden regressions. Cost is confined to modes that really are blank: plugins that draw immediately are unchanged (clock-simple and tide-display still 2s), while on-air -- eight deliberately blank modes -- goes to 21s. Closes #527. * fix(harness): load nested schema defaults, and merge caller config at leaf level load_config_defaults read only top-level properties. An object property carries its defaults on its children, not on itself, so everything nested was dropped -- 2,386 defaults across 37 of 44 plugins, soccer-scoreboard alone losing 539 of 565. render_plugin_matrix's comment says the plugin then "behaves like a real install", which for most of the fleet it did not. _defaults_from_properties now recurses. merge_config deep-merges the caller's config onto the result so an override lands at the leaf: a shallow merge would let -c '{"nhl": {"enabled": true}}' replace the whole nhl subtree and discard every other nhl default, which is the same class of bug being fixed here. Measured before/after across all 49 installed plugins on the rig: **no render changed** -- identical PASS/FAIL counts, byte-identical output, goldens intact. Plugins already fall back to the same values internally via config.get(key, default), so supplying them explicitly agrees with what they were doing. The defaults really are arriving now: ufc-scoreboard 9 -> 87 defaults ledmatrix-flights 51 -> 95 masters-tournament 10 -> 51 cricket-scoreboard 22 -> 50 tide-display 12 -> 18 and hockey-scoreboard, which used to load nhl.enabled=None, now gets nhl.enabled=True with its full display_modes block. Caveat worth carrying: the eight plugins with the most nested config (soccer, baseball, basketball, hockey, lacrosse, football, afl, nrl -- 1,634 of the 2,386 dropped defaults, 68%) could not be measured. They import src.common.sports_shared, which the test rig's core branch predates, so they fail to load there identically before and after. Re-run this comparison against a core that has that module before trusting the "nothing changed" result for them; those are exactly the plugins whose renders should change most. Closes #531. * refactor: narrow the exception handlers this branch introduced Codacy flagged the new code; it passes on other recent PRs, so the finding is mine. Four of the five broad `except Exception` clauses I added were catching far more than they needed to, which is the same shape as several bugs this branch fixes -- hello-world's TypeError sat invisible for exactly this reason. freezer() / move_to() / tick() -> (AttributeError, TypeError, ValueError) cache_manager.delete() -> (OSError, AttributeError, KeyError) The fifth stays broad and now says why: it wraps a call into a plugin's own display(), which can raise anything, and the first frame has already rendered -- so a failure there must not turn a good result into an error. Verified against a checkout of main: f1-scoreboard 48 PASS / 0 FAIL with 0 empty warnings, on-air keeps its 8 true positives, clock-simple 8 PASS. geochron shows 7 golden drifts both before and after this branch, so it is not from these changes -- its committed goldens predate #521's 1-bit text rendering. * fix: resolve CodeRabbit review and Codacy findings on #534 CodeRabbit raised six; all six were real. The test double had drifted ahead of production. VisualTestDisplayManager accepted set_scrolling_state(frame_hold=...) while DisplayManager did not, so such a call passed every harness run and would raise TypeError on the panel -- the one failure a safety harness exists to prevent. frame_hold belongs to the change that adds it to DisplayManager (#523), so it moves there and the double matches main again. The harness swallowed exceptions from re-rendered frames. _settle_loop re-renders a mode that came back blank, to give a scroll time to draw; returning silently on a crash meant a mode that renders one good frame and then explodes was reported as passing. Recorded on result.error now, keeping the captured frame so the failure stays inspectable. starlark-apps display() returned True after _display_frame() failed, so the controller held a dead frame for the whole display_duration instead of rotating on. _display_frame now returns bool on all three paths. run_plugin_tests.py used env.setdefault for PYTHONPATH and LEDMATRIX_CORE, so an inherited value won and the subprocess imported a different core than the one under test -- ledmatrix-plugins#467 exactly. Prepends PROJECT_ROOT and sets LEDMATRIX_CORE unconditionally. The on-demand mailbox is polled after every frame, ~125x/second on a scrolling mode, and the read is deliberately uncached, so it was that many disk reads per second to find nothing. Floored at 250ms, which is imperceptible for a web-UI click. Consuming it also deleted whatever was present rather than what had just been processed, so a request posted while the previous one was in flight was thrown away and never ran; the delete is now keyed by request_id. That narrows the window rather than closing it -- a true atomic claim needs a primitive the cache layer does not offer, and the code says so rather than implying otherwise. Codacy's 2 criticals were bandit B404/B603 on the subprocess call added to run_plugin_tests.py. Fixed interpreter, argument list, no shell; annotated with the repo's existing nosec convention. Bandit is clean on the file. Adds test/test_on_demand_mailbox.py (8), test_starlark_display_contract.py (4) and two settle cases in test_harness_empty_claimed.py. 4, 4 and 2 of those fail against the pre-fix code. Full suite: 3961 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 * chore: satisfy Codacy's subprocess checks on the new test runner Codacy runs Bandit and Opengrep (its Semgrep fork). The new subprocess.run in scripts/run_plugin_tests.py trips three patterns, on two different lines: Bandit B404 on the import, B603 on the call Opengrep dangerous-subprocess-use-audit on the run( line dangerous-subprocess-use-tainted-env-args on the argv line A nosemgrep applies only to its own line, so the call line and the argv line each need one; a single comment on the call covered neither rule fully. Suppression is the right answer here rather than a rewrite: the interpreter is sys.executable, the arguments are a list, and no shell is involved, so there is nothing to word-split or expand. Matches the pair the rest of the repo already uses for this shape -- permission_utils.py, plugin_loader.py, install_dependencies_apt.py. Codacy: 0 new issues, up to standards. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 * chore: leave visual_display_manager untouched so #523 can merge The only change this branch made to that file was a docstring, and it collided with #523's rewrite of the same method -- so #534 and #523 each merged cleanly against main but conflicted with each other. Reverted to main's text; #523 owns this method and adds frame_hold to it. The note the docstring carried ('frame_hold arrives in #523') would have been stale the moment #523 landed anyway. The parity test in #523 is what actually keeps the two signatures honest. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 * chore: add the Ruff suppression nosec/nosemgrep do not cover Ruff reports S603 on the same call Bandit and Opengrep do, and none of the three suppressions covers the others. Confirmed the precondition first: path comes from discover_plugin_tests(), which globs test files inside the repo, and the call is a fixed interpreter with a list argv and no shell. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-07 13:37:38 -04:00
ChuckandClaude Opus 5	91d15a8943	fix(display): draw text 1-bit, so glyphs stay crisp on the LED grid (#521) ... An LED panel has no partial brightness. PIL defaults ImageDraw's fontmode to "L", which anti-aliases TrueType glyphs into a grey fringe the panel can only round off -- a 4px glyph arrives smeared into 3px. DisplayManager creates its shared `draw` in six places and set fontmode at none of them, while _load_fonts loads extra_small_font as 4x6-font.ttf at size 6. Measured at draw time, that face at that size puts 74% of its lit pixels at partial coverage. Every plugin drawing small text through the shared draw inherited the blur; geochron was the case that surfaced it. The harness's VisualDisplayManager had the same gap, which mattered more than it looks: goldens were recording anti-aliased text that production would not produce, so the harness could not have caught this. Fixing only production left geochron still blurry under the harness -- that is how the second site was found. Both are set to "1" so the harness renders what the panel renders. Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-04 16:02:04 -04:00
Chuck	32d637a446	fix(store): read the core version from disk, not from a stale import (#518) ... * fix(store): read the core version from disk, not from a stale import Updating the core to 3.3.0 and then updating plugins refused all eight sports scoreboards: Refusing to install nrl-scoreboard: NRL Scoreboard supports LEDMatrix >=3.3.0, but this system is running 3.2.0. while src/__init__.py on that machine read 3.3.0. Observed on hardware, not theorised. The gate ran `from src import __version__ as core_version`, which binds whatever the process loaded at start. The plugin store's gate lives in the web UI, a long-lived service of its own, and the update route deliberately restarts nothing -- it replaces files on disk and asks the user to restart. Its prompt named only the *display* service, so a user who followed it left the web process holding the previous number. Stale by exactly one release is the case that bites: every plugin flooring on the release you just installed is refused, blaming a core version that is already correct on disk. It reads as a broken plugin store. 3.3.0 is the first release where this hits a whole family at once, since all eight scoreboards floor there. compatibility.current_core_version() reads the version from the file instead, falling back to the imported value on any failure -- so it can only ever be as correct as before, never worse. All four gate call sites use it: three in store_manager (install, the git-pull update path, install_from_url) and one in plugin_loader's advisory warning. The restart prompt now names both services. Twelve tests, including the hardware failure itself: a process holding 3.2.0 while disk says 3.3.0 refuses hockey, and reading fresh allows it. The inverse is asserted too -- a genuinely old core still refuses, so the gate has not become permissive. One test greps both modules for the old import-bound read; reintroducing that line fails it, which is what stops this coming back. Not changed: web_interface/__init__.py also imports __version__, but for display rather than gating, and the API endpoint already reports a fresh git describe. * fix: drop the unused os import Left over from a first draft that joined paths by hand before this used pathlib. Flagged by CodeRabbit on #518; confirmed dead -- no os. reference remains in the module.	2026-09-03 16:06:40 -04:00
ChuckandClaude Opus 5	a686932c7e	fix(store): land the install_from_url gate, which never reached main (#511) ... #510 shows as merged, but into fix/gate-git-pull-updates -- #508's branch -- rather than main. #508 reached main first, so the sideload gate was left behind on a branch. Same failure as plugins #350/#351, which merged into each other's bases; worth knowing the pattern, because GitHub reports these as MERGED and `gh pr list` shows nothing outstanding. main today has two of the three routes gated: install_plugin (#431/#433) and update_plugin's git branch (#508). install_from_url validates required manifest fields and then installs whatever it found, never comparing the core version. Cherry-picked unchanged from the orphaned branch -- it applies to main with no conflict. TestSideloadGate pins the three cases the other routes pin: refuses a floor above this core leaving nothing behind, still allows a compatible plugin (the guard against a gate that refuses everything), and does not block a 2.0.0 floor on a core reporting an untrustworthy version. Full suite 3725 passed, 6 skipped. Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-02 09:51:27 -04:00
ChuckandClaude Opus 5	9b522d412c	fix(store): gate the git-pull update path (#508) ... * fix(store): gate the git-pull update path `install_plugin` gates every route that re-downloads, `_reinstall_with_rollback` included. `update_plugin` has one branch that re-downloads nothing: a git checkout pulls in place, installs dependencies, and returns True. A pull could therefore deliver a manifest flooring above this core and nothing would notice until the plugin failed to load — which surfaces as one line in the journal and a display that silently stopped appearing. Checked after the pull rather than before it, for the same reason `_install_plugin_impl` checks after the download: the registry carries no compatibility field, so the incoming floor is only knowable once the new commit is on disk. Undone with `git reset --hard` to the pre-pull commit rather than by removing the directory. This is a live checkout, the old commit is still in the object store, and the reset leaves the user on the exact version they were already running — the same promise `_reinstall_with_rollback` makes, reached by the means this path actually has, with no window where the plugin directory does not exist. An unreadable manifest allows: it is not evidence of a floor. Scope, stated plainly: monorepo plugins install as archives and update through `_reinstall_with_rollback`, so they were already gated. Only registry entries with no `plugin_path` reach this branch. It is closed anyway because the sunset rule in the plugins repo's `08-shared-sports-code.md` names, as condition 3, that the core enforces the floor "at install/update time" — and B6 rests on that being true rather than merely written down. `install_from_url` is still ungated; the tests say so rather than letting the next reader assume otherwise. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 * fix(store): do not pull what the gate cannot un-pull Review of the gate found a data-loss path it had introduced, plus two smaller scope errors. All three from CodeRabbit on #508. **The stash failure was load-bearing and was not treated as one.** update_plugin stashes local changes before pulling; when that stash failed or timed out it logged a warning and pulled anyway. That was harmless while nothing ever undid a pull. It is not harmless now: the gate's rollback is `git reset --hard`, which discards uncommitted tracked edits -- exactly the edits the stash existed to protect. A pull does not refuse on a dirty tree as long as the incoming commit touches other files, so the sequence completed silently: pull succeeds, gate refuses, reset takes the user's work with it. update_plugin now returns before pulling unless the tree was already clean or was successfully stashed. Refusing costs an update in a case that had already gone wrong; the alternative costs data. That also makes `--hard` safe by construction in _gate_pulled_commit, and its comment now says so rather than observing it in passing. Pinned by test_a_failed_stash_stops_the_update_before_pulling, which writes a local edit, forces the stash to fail, and asserts both that HEAD did not move and that the edit is still on disk. Verified it bites: with the new guard removed the file comes back as `class P: pass`, the edit gone. **_HAS_GIT could take the module down instead of skipping it.** With no git on PATH, subprocess.run raises FileNotFoundError, and this runs at import time -- before skipif can act, so the whole file errors rather than skipping. Now catches OSError. **The doc overclaimed the gate's reach.** It said the floor is enforced on "every route that installs or updates" while the same passage notes install_from_url is ungated. Both spots now scope the claim to registry-managed installs and the two supported update paths, and name the sideload exception. Full suite 3720 passed, 6 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-31 17:35:00 -04:00
ChuckandClaude Opus 5	bdced206dc	fix(plugins): retain state history by age, with the count as a ceiling (#502) ... * fix(plugins): retain state history by age, with the count as a ceiling Follow-up to the cap in this PR. A flat entry count answers the wrong question: what a reader wants from this history is "the last couple of hours", and how many transitions that is depends entirely on the plugin's update interval. On a real board those span 2s to 3600s, so 200 entries is interval 200 entries covers 2s 3.3 minutes (flights, live) 10s 16.7 minutes (jellyfin) 60s 1.7 hours (default) 300s 8.3 hours (news) 3600s 4.2 days -- the plugin churning hardest, the one worth looking at, keeps the least. So transitions are now trimmed by AGE first (STATE_HISTORY_MAX_AGE_SECONDS, two hours), which makes the retained window comparable whatever the cadence, and the count cap becomes purely a memory ceiling for pollers fast enough to exceed it inside that window. The ceiling rises 200 -> 2000: at ~230 bytes an entry that is ~0.5MB per plugin worst case, and only plugins updating faster than roughly every 4s can reach it. Steady-state memory is unchanged for everything slower, since the age trim binds first. Two details worth stating: - The trim reads time.monotonic(), stored alongside each transition, rather than the datetime already inside it. A DST shift or an NTP step would otherwise make every entry look ancient and flush the history in one go. The human-readable timestamp is untouched and still what get_state_history() returns. - Trimming happens on append, so a plugin that goes quiet keeps its last window until it writes again. That is deliberate: it is bounded either way, and a lazy trim costs nothing on the hot scheduling path. The guarantee is therefore about the SPAN of retained history, not its age against the current clock, and the test asserts it that way. The public shape is unchanged: get_state_history() still returns the same list of transition dicts, and state_history_count is still the lifetime total. test_plugin_state_history_retention.py adds 7 tests. Verified against this branch with only the age trim removed: 4 fail, 3 pass -- the three that survive are testing the count ceiling and the monotonic clock, which this commit does not change. Full suite 3753 passed, 60 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW * fix(plugins): build get_state_info() as one locked snapshot Every field was read under its own lock, so an unload running concurrently could be observed half-done: 'state' read before clear_state() removed it and 'state_history_count' read after, handing PluginManager.get_plugin_info() a plugin that is ENABLED with zero transitions. The whole payload is now built in one critical section. _lock is an RLock, so the helpers called inside it can still take it. The regression test runs a reader against a thread that repeatedly fills and clears the same plugin, and fails on the first torn snapshot. Verified by removing only the lock: fails on 3 of 3 runs, passes on 3 of 3 with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-26 08:56:45 -04:00
Ron PierceandClaude Opus 5	39e7f8cbe0	fix(plugins): cap the per-plugin state transition history (#501) ... * fix(plugins): cap the per-plugin state transition history PluginStateManager recorded every state transition in a per-plugin list and never trimmed it. The only code that removed entries was clear_state(), called solely from PluginManager.unload_plugin(), so a plugin that stays loaded -- normal operation -- never released one. The list is written on the hot scheduling path. Every update cycle appends twice: _reserve_for_update() sets RUNNING and _finish() sets ENABLED back again. At the default 60s update interval that is 2,880 entries per plugin per day, and nothing reads them -- get_state_info() only takes their len(). Pure dead weight. Measured against the unpatched class, ten plugins on a 60s interval: sim uptime history entries heap growth 1 day 28,810 7.7 MB 7 days 201,610 53.9 MB 30 days 864,010 230.9 MB (still climbing) With the cap it is flat at 2,000 entries / 0.5 MB from day one. On a 1 GB board 231 MB of garbage is fatal on its own, and the failure is not a clean OOM: once MemAvailable falls far enough fork() starts returning ENOMEM, so sshd accepts connections and closes them before its banner while the kernel still answers pings. The board looks like a hardware fault and needs a power cycle. Same family as the ceilings added in #464. Retain the most recent 200 transitions per plugin in a deque and let the rest age out. state_history_count is surfaced through the web API, so the lifetime total is tracked separately rather than plateauing at the cap. get_state_history() now returns a copy under the lock; it was handing out the manager's own list, which a caller could mutate. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(plugins): copy history entries out, lock clear_state Review follow-ups on the transition history. get_state_history() copied only the outer list, so a caller holding a returned transition could rewrite the manager's record of what happened -- which contradicted the defensive-copy guarantee in its own docstring. Copy each entry too. Every value in a transition is immutable, so a shallow copy per entry is enough. test_get_state_history_entries_are_copies pins it; without the change it fails with 'tampered' == 'enabled'. clear_state() mutated five shared dicts without holding _lock, while every other mutator takes it. A concurrent set_state() could interleave and leave a plugin with history but no state. Drop the five as one unit. This does not close the wider unload-vs-worker race, which lives in PluginManager.unload_plugin() and predates this change: an update worker still in flight can call set_state() after clear_state() returns and recreate the entry. Serialising that needs the per-plugin lock held across worker join in unload_plugin(), which is a separate change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-08-25 09:45:17 -04:00
ChuckandClaude Opus 5	c321b94085	fix(display): retry a plugin that is enabled but failed to load (#495) ... * fix(display): retry a plugin that is enabled but failed to load A plugin whose validate_config() returns False is treated as a hard load failure. The API then reports enabled=true, loaded=false, error=null: the plugin is simply absent, with nothing saying why. hockey-scoreboard sat in that state on a live rig for four days. The recovery path existed but could not be reached. _reconcile_enabled_plugins computes to_add = desired - current, and a plugin that failed to load is never in current, so it stays in to_add and would be retried. But the reconcile is queued by _enabled_set_changed(), which compares only top-level `enabled` flags -- and the edit that actually fixes such a plugin (enabling a league, filling in an API key) is nested inside the plugin's own config section. No top-level flag changes, so no reconcile is queued, and the save that should have fixed it does nothing. Only toggling some unrelated plugin -- which does change a top-level flag -- queues the global reconcile that recovers it. Add a second gate: queue a reconcile when a discovered plugin is enabled in config but absent from the running set. It is deliberately narrow rather than "reconcile on any config change". Reconcile calls discover_plugins(), a ~39-manifest filesystem scan, and it runs on the render thread; doing that on every config save would trade this bug for a frame hitch. Gating on plugin_manifests also keeps non-plugin sections that carry their own `enabled` flag (schedule, display) from queueing a reconcile they can never satisfy. In the steady state -- every enabled plugin loaded -- the new check is False and costs nothing. The same valid-but-unconfigured => hard-fail shape still exists in text-display, youtube-stats, birdnet-go, ledmatrix-flights and mqtt-notifications; this makes all of them recoverable without a restart. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW * fix(display): snapshot the plugin mappings under their locks Addresses the review finding on the cross-thread reads. _enabled_plugin_not_running runs on the config-watcher thread and read two mappings the render thread mutates. Catching RuntimeError was not a fix: it turned a torn read into a coin flip between an unnecessary discovery scan and a missed retry, which is the bug this PR exists to remove. Both reads are now snapshots taken under the lock that guards their writes: - plugin_manifests via a new PluginManager.discovered_plugin_ids(), which copies the ids while holding the existing _discovery_lock. Discovery rebuilds that mapping entry by entry, so an unsynchronised reader can see it half-populated. - plugin_display_modes under a new controller lock, taken at the only two sites that mutate it (_register_loaded_plugin / _unregister_plugin). The locks are never nested -- each snapshot is taken and released before the next -- so this cannot deadlock against discovery, which holds _discovery_lock while it rebuilds. No cost on the per-frame path. Both mutation sites run during reconcile, which is rare, and every hot-path read of plugin_display_modes is on the render thread itself, same thread as the writes, so those stay lock-free. Tests: the accessor returns a snapshot rather than a live view, and actually takes the discovery lock (proved from a second thread, since an RLock is reentrant on the owning one) so a later refactor cannot quietly drop it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW * fix(display): consume the reconcile request before serving it Addresses the second review finding: a lost update on _pending_plugin_reconcile. The flag was cleared after a successful reconcile. Reconcile has already read its config by that point, so a config change arriving mid-flight set a flag that the trailing clear then erased -- a request that was never served, and the newest config never reconciled. That is the same "my save did nothing" symptom this PR exists to remove, so leaving it would have undercut the fix. Consume the request before running it instead, and re-arm only on a retryable failure. A change that lands during reconcile now stays set and is picked up on the next pass. The per-frame read stays lock-free. It is a fast path that can only produce a false negative -- the watcher setting the flag just after it is read is seen on the next iteration -- never a false positive that loses a request. The lock is taken only when a reconcile is actually pending or a config change arrives. Extracted _service_pending_reconcile() so the sequence is testable rather than buried in run()'s loop; the review asked for a regression test that invokes the subscriber during reconciliation, which is not reachable otherwise. Tests: 4 new, covering a request racing in mid-reconcile, the quiet success, the retryable-failure re-arm, and not reconciling when nothing is pending. Two of them fail against the previous clear-after-success semantics. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-23 11:43:39 -04:00
ChuckandClaude Opus 5	5f29243e87	fix(config): make the device location the default for plugin location fields (#490) ... * fix(config): make the device location the default for plugin location fields A user in Kansas City reported their radar centred on Dallas, TX with nothing in config.json to explain it. The radar is the `ledmatrix-weather` plugin's `radar` mode, and it centres on the same coordinates as every other weather mode: `forecast_data` lat/lon, geocoded from the plugin's own `location_city` / `location_state` / `location_country`. Those ship with schema defaults of Dallas / Texas / US. A user who never opened the weather plugin's config form therefore has no `location_city` on disk, and `PluginManager` merges the schema default in at load time — so the whole plugin (not just the radar) silently runs on Dallas. Radar is just the only mode that draws a recognisable map and gives the mismatch away. Meanwhile the device-wide `location` block that General settings writes was read by nothing at all, despite its own help text promising it was "used for weather, sunrise/sunset, and other location-based content". `SchemaManager.generate_default_config()` now substitutes the device `location` into the three fully-namespaced `location_*` keys before handing defaults back, so the promise holds: - Only `location_city` / `location_state` / `location_country` are substituted. A bare `state` key is left alone — `ledmatrix-elections` uses it for a two-letter code, and rewriting it would break that plugin. - A value the user saved on the plugin still wins: this replaces the schema default, and `merge_with_defaults` puts user config on top. - The substitution is applied on the way out of the defaults cache rather than into it, so changing the device location takes effect immediately. - No config manager, no `location` block, or an unreadable config all fall back to the plugin's own schema defaults. Every caller benefits: the plugin loader, the config form (which now pre-fills the user's real city), config save, and reset-to-defaults. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GNLrSZ32FNKpHRaduKEJsg * docs(web): name the exact plugin keys the device location seeds Review follow-up. The General settings help text said the device location was "the default for every plugin that asks for a city", which overstates what the code does: only the fully-namespaced `location_city` / `location_state` / `location_country` keys are substituted. A plugin with a bare `city` key gets nothing — deliberately, since `ledmatrix-elections` uses `state` for a two-letter code. The tips now name the exact keys. Worth noting for anyone editing these: `ui.help_tip(...)` takes a single-quoted Jinja string, so an apostrophe in the tip text has to be escaped or written around. The wording here avoids them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GNLrSZ32FNKpHRaduKEJsg --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-08-21 16:22:48 -04:00
ChuckandClaude Opus 5	1fbe244e49	fix(plugins): say when discovery skips a directory (#489) ... * fix(plugins): say when discovery skips a directory A plugin can be enabled in config, enabled in plugin state, present on disk with a valid manifest and an importable entry point -- and simply absent from the running process, with nothing anywhere to say why. That is not hypothetical. hockey-scoreboard on a live rig is enabled in both places, imports cleanly when loaded by hand, and is listed in the Vegas plugin order, but is not among the 22 plugins the process actually holds. Establishing even that much meant comparing cache-file mtimes to find it had last run three days earlier. The journal had nothing, because discovery does not report what it declines to load. Two paths were silent. A directory with no manifest.json was skipped without comment, which is defensible until it is the thing you are trying to explain. Quieter still, a manifest that parsed but carried no "id" was read successfully and then dropped on the floor -- no warning, no trace, and the plugin simply does not exist as far as the rest of the system is concerned. Both now log a warning naming the directory and the reason. This does not explain the rig above; its manifest has an id. It makes the next occurrence diagnosable from the journal instead of from file timestamps. Reverting the change fails both tests. 65 plugin-system tests pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW * fix(plugins): warn once per directory, not once per scan Self-review catch. Discovery runs on every web UI page load and every config reconcile, so warning unconditionally about an unloadable directory would put a line in the journal each time someone opened a page -- the same log-volume problem this change exists to help diagnose. The skip is now reported once per directory per process. The diagnostic value is unchanged: the reason a plugin is missing still appears in the journal, once, where before it appeared nowhere. Test added covering five consecutive scans producing one warning. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW * fix(plugins): one unusable manifest no longer aborts the whole scan json.load accepts any JSON value, so a manifest.json holding null, [], "text" or 42 parses without complaint and then raises AttributeError on manifest.get('id'). Nothing catches that: the outer handler around the scan takes OSError and PermissionError only. So a single malformed manifest did not skip that one directory -- it aborted _scan_directory_for_plugins outright, and every other plugin on disk, however healthy, silently failed to register. Reproduced with three directories, the middle one holding `null`: SCAN ABORTED -> AttributeError: 'NoneType' object has no attribute 'get' the two valid plugins never registered That is the same failure this PR set out to fix, in its most severe form: a plugin enabled in config, enabled in plugin state, present on disk, and absent from the running process with nothing to say why -- except here it takes every other plugin with it. A manifest that is not a JSON object is now skipped like any other unusable directory, named once, with what it actually was: Skipping bad-null: its manifest.json is NoneType, not a JSON object Skipping bad-list: its manifest.json is list, not a JSON object scan returned: ['aaa-good', 'zzz-good'] Verified: removing the guard fails 6 of the 10 tests. Covers null, list, string, int and bool, and asserts the healthy plugins either side of the bad one still register. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-21 16:19:33 -04:00
ChuckandClaude Opus 5	863e4a1ecd	consolidate(perf): cut SD writes, log volume, and metrics churn (#486) ... * fix(plugins): one bad metrics cache entry should not stop every plugin Caught live on a rig: every plugin failing, once each, continuously. ERROR - src.plugin_system.plugin_manager - plugin geochron operation failed: ResourceMetrics.__init__() got an unexpected keyword argument 'consecutive_failures' ERROR - ... plugin text-display operation failed: ... ERROR - ... plugin news operation failed: ... ERROR - ... plugin odds-ticker operation failed: ... with /api/v3/health reporting plugin_system: not_initialized while the display process itself kept running and updating the panel. `consecutive_failures` is a plugin_health field, not a metrics one. get_metrics() does ResourceMetrics(**cached), which raises TypeError on a single unrecognised key, and that exception escapes into plugin_manager and is reported per plugin. One malformed cache entry takes the whole plugin system down. How a health-shaped record came to sit under a plugin_metrics key on that machine is not established, and I could not finish the diagnosis: the rig went back into its EIO failure mode partway through -- SSH resetting pre-banner, systemctl unexecutable -- while the web API kept answering from RAM. Checked before that: the cache files on disk are correctly shaped and separate, and CacheManager.get() returns the right record for each key, so it is not a live key collision. A restored backup mixing two machines' caches is the likeliest explanation, and that rig had one restored onto it. Either way the loader should not be brittle enough for the answer to matter. plugin_health already repairs its records field by field rather than trusting what is on disk; this does the same. Known fields are kept, unknown ones are dropped and named once in the log so a genuine schema change stays visible rather than being silently discarded, and a non-mapping entry no longer raises. Keeping the known fields matters: discarding the record wholesale would throw away real call counts and timings because of an unrelated stray key. Mutation-checked: restoring ResourceMetrics(**cached) fails 6 checks, dropping the whole record fails the field-preservation check, and dropping unknown fields silently fails the logging check. 28 tests pass across the resource monitor and plugin health suites. * perf(health): stop rewriting a health record on every healthy cycle Every successful plugin update called record_success(), which persisted the record unconditionally. In steady state the only fields that had changed were total_successes and last_success_time -- a counter and a timestamp that health_monitor surfaces for display and that nothing reads back after a restart. Nothing alerts on the age of last_successful_update; it is carried in the metrics dataclass and shown. Measured on a rig running 24 plugins, all steady-state (0 consecutive failures, circuit closed): a five-minute sample caught 22 health-file rewrites, about 4.4 a minute or 6,300 a day. Each write is ~400 bytes through cache_manager.set(), which writes a file per call, so each one costs a filesystem block plus an ext4 journal write. That lands on an SD card, where the unit of cost is an erase-block cycle rather than the bytes involved, and where wear is what eventually kills the card. Two cards have already failed on the other rig with the same signature -- unreadable block device, EIO on exec, sshd unable to read its host keys. The circuit breaker still has to survive a restart, so the write is kept for exactly the fields it is rebuilt from: consecutive_failures, circuit_state, circuit_opened_time, half_open_start_time. A failure, a circuit opening and a recovery are all still written the moment they happen. In-memory state is updated every time either way, so the health API and web UI show what they always did. Tested: 100 healthy cycles now perform zero writes after the first, the counters remain accurate in memory, and a failure, a recovery and a half-open-to-closed transition each still reach disk. One test kills and rebuilds the tracker from the cache to prove the breaker's state genuinely survives what is no longer written. Mutation-checked both ways: persisting unconditionally again fails the steady-state test, and widening _DURABLE_FIELDS to include last_success_time fails it too. The 46 existing health tests pass. (cherry picked from commit 14abea2d24) (cherry picked from commit 0f77bd2345) * perf(vegas): trace the content path at DEBUG instead of INFO plugin_adapter narrates every step of acquiring content from every plugin -- "Has get_vegas_content", "Native: calling get_vegas_content()", "Native content returned None", "Has scroll_helper", per-item sizes -- once per plugin per cycle, all at INFO. Measured on a live rig: 13,408 log lines an hour, of which 13,366 were INFO and 35 were WARNING. Roughly 223 lines a minute of string formatting on a Pi that is also driving the panel, written through journald to the SD card, with the 35 lines that actually indicate a problem buried among them. Top repeated messages in that hour: 717 Scroll progress: elapsed=... total_scrolled=.../... px 399 [plugin] --> INCLUDED in Vegas scroll 323 [plugin] content_type=static, display_mode=fixed 195 [plugin] Has get_vegas_content: True 195 [plugin] Native: calling get_vegas_content() 168 [plugin] Native: get_vegas_content() returned None 168 [plugin] Native content returned None <- the same fact, twice 54 logger.info calls in plugin_adapter become logger.debug, along with the per-frame scroll-progress line in scroll_helper. Together those are 3,174 of the 13,408 lines an hour, a 23% cut, and the ~3,600 odds-manager lines are addressed separately by ledmatrix-plugins#300. Nothing is lost: the 19 warning/error/exception calls in the module are untouched, so real failures still surface at their own level. This is a logging-level change only -- no control flow, no behaviour. One INFO call is deliberate and stays. The padding-strip message picks its level at runtime (`logger.warning if (left and right) else logger.info`) and test_vegas_plugin_adapter.py pins that choice; it survives because it is not a direct logger.info call site. That test still passes. Mutation-checked both ways: reintroducing a single INFO trace fails the guard, and demoting the warning/error calls along with the trace fails a second guard written for exactly that mistake. 537 vegas and scroll tests pass. (cherry picked from commit e496d95dfe) (cherry picked from commit 8d1e43c15a) * fix(logging): give the journal the real severity of each line Everything this process writes to stdout reaches the journal as PRIORITY=6, whatever the Python level was, because journald has nothing else to go on. Measured on a live rig over 24 hours: lines containing " - ERROR - " 55 lines containing " - WARNING - " 13 journald PRIORITY recorded 6, for every one of them So `journalctl -p err -u ledmatrix` returns nothing while errors are being logged, and `-p warning` likewise. Triage falls back to grepping message text, which is slower and unreliable: during this audit a search for "oom" matched the radar logging "zoom=9" twenty-four times and briefly looked like the OOM killer had been firing. systemd reads a leading "<N>" on each stdout line and takes it as the priority (sd-daemon(3)), so a formatter that prefixes one costs no dependency. Every line of a multi-line record is tagged, not just the first -- the journal splits them, and an untagged continuation reverts to the default, which would leave the body of a traceback filed as informational while its first line was an error. Applied only when JOURNAL_STREAM is set, which systemd sets for services whose output it captures. Run from a terminal, in the emulator or under pytest the prefixes would be literal noise, and the file handler keeps the plain formatter for the same reason. Mutation-checked three ways: prefixing unconditionally fails the outside-systemd test, prefixing only the first line fails the multi-line test, and mapping ERROR to 6 fails the level mapping. 39 tests pass across the logging suites. (cherry picked from commit 780fca6365) * fix(logging): let callers see through the journald formatter wrapper CI caught what local testing could not: two existing tests in test_logging_config.py assert that setup_logging() selected a StructuredFormatter or a ContextualFormatter, by checking the console handler's formatter directly. Wrapping that formatter to tag each line with its syslog priority makes those assertions false. They passed locally and failed on the runner because the wrapper is applied only when JOURNAL_STREAM is set -- absent in a terminal, present in CI. An environment-dependent break, which is the kind that gets shipped. The wrapper now exposes the formatter it delegates to, and those two tests look through it. They are about which formatter format_type selects, and that behaviour is unchanged; only the object they have to reach for moved. Verified both ways this time: 39 tests pass with JOURNAL_STREAM set and with it unset. * perf(plugins): stop rewriting a plugin's metrics file on every call Plugin metrics were persisted to the cache inside monitor_call, so every call by every plugin rewrote a small JSON file. Measured on a running rig: one plugin's plugin_metrics file changed nine times a minute, with fourteen such files active. Each is around 350 bytes, which on ext4 costs a 4KB block plus a journal entry, so the cost is dominated by the write itself rather than the payload. Cache writes accounted for essentially all of that device's 2.4 MB/min of SD traffic, on a card that wears out and has already failed twice on the other rig. Metrics cannot be de-duplicated the way health state can, because call_count changes on every call and the timings usually do too. So they are rate-limited instead: at most one write per plugin per 30 seconds. The in-memory copy stays authoritative and exact -- a plugin's call_count is still precise the instant after it runs. Only the cross-process snapshot the web UI reads is delayed, and telemetry up to half a minute old is still a fair description of a long-running plugin. reset_metrics clears the throttle timestamp, so a reset is not left showing a deleted key for the rest of the interval. Extrapolating the sampled rate, this takes metric writes from roughly 126 a minute to 28. Health persistence, the other half of the churn, is handled separately in #475. Verified by reverting the throttle: the churn test then reports 50 writes for 50 calls. 88 tests pass across resource monitor, plugin system and web API. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW * fix: use a monotonic clock and only mark metrics persisted once written Two review findings on the throttle, both right. The interval compared wall-clock timestamps. These devices have no RTC, so the clock jumps by however far off boot-time was the moment NTP first syncs -- a forward jump would allow an early write, a backward one would stall the snapshot well past the interval. time.monotonic() is not subject to either. The timestamp was also recorded before cache_manager.set(). A set() that raised would buy the next interval's silence without leaving a snapshot behind, which is the one case where skipping the write is least affordable. Recorded after the write lands instead, so a failure is retried on the next call. Verified by restoring the original ordering: the new test then reports one write where two are expected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW * Address all six review findings on the perf consolidation CodeRabbit reported six; this is all six, checked against its own "Actionable comments posted: 6" rather than against what I happened to scroll past. Three are real defects in the code: 1. _under_systemd() trusted the presence of JOURNAL_STREAM. systemd publishes JOURNAL_STREAM as "dev:ino", and every child process inherits it -- including one whose stdout has been redirected to a pipe or a file. The variable outlives the descriptor it describes, so a subprocess would decide it was talking to the journal and emit the "<N>" priority prefixes as literal noise into that captured output. That is exactly the noise the function exists to prevent. It now parses the pair and fstats stdout, per systemd's own guidance, and returns False for missing, malformed, mismatched, or unusable descriptors. 2. Cached metrics were not type-checked. A dataclass does not enforce its annotations, so ResourceMetrics(call_count="not a number") builds happily and only fails later, deep inside monitor_call: TypeError: can only concatenate str (not "int") to str Values are now coerced to their declared type at load, where there is still a cache key to name in the warning, and a value that cannot be coerced starts the plugin fresh instead of arming a delayed failure. A numeric string is accepted rather than discarded -- a JSON round-trip can widen an int, and that is recoverable. 3. The first metrics snapshot was skipped for the first 30s of uptime. _persist_metrics used 0.0 as the "never written" default. monotonic() is time since boot on Linux and systemd starts this service at boot, so `now - 0.0 < 30` was true for the first half-minute of every run: the throttle swallowed the very first write, the one that matters most after a restart. The sentinel is now None and the interval is only applied when a previous write exists. Three are tests that could pass without testing anything: 4. test_health_write_churn's fake cache stored by reference, so the tracker kept mutating the object already in the store -- a record could look persisted when no write had happened, which is precisely what test_durable_state_survives_a_restart exists to detect. Both directions now deep-copy, like a cache that serialises to a file. Verified: disabling the one real cache write now fails three tests. 5. test_values_of_the_wrong_type_do_not_raise asserted only that a dataclass had been constructed, which was true with the bad value still in it. It now asserts the loaded metrics are usable -- the field is numeric, and arithmetic on it does not raise -- across four kinds of bad value. 6. test_vegas_log_volume counted "logger.error(" in the source text, which also matches comments, docstrings and string literals -- including that module's own docstring, which names those levels. A real error call could be demoted with the tally unmoved. It now walks the AST, reusing the helper already in the file. Verified: demoting all 18 warning/error/exception calls now fails the test. Verified: every fix mutation-checked by reverting it and confirming the matching test fails. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-21 15:50:55 -04:00
ChuckandClaude Opus 5	0c5b9c57d3	fix: keep low-memory boards reachable under load (#464) ... * fix(service): survive corrupt health cache and clean exits Three independent failure modes that each end with a dark panel and no automatic recovery. 1. PluginHealthTracker._load_health_state returned the cached value verbatim. If that value is not a dict, every caller raises AttributeError: 'list' object has no attribute 'get' — during DisplayController.__init__, so the process dies before the display loop starts. systemd restarts it, the same bad entry is read back from disk, and it dies again: an unattended restart loop that survives reboots because the cause is persisted. Observed in the field with plugin_health:<id> holding an unrelated plugin's list payload. Now non-dict entries are discarded with a warning and the defaults are rebuilt. 2. ledmatrix.service used Restart=on-failure, so any exit with status 0 left the unit stopped and the panel dark indefinitely — systemd treats it as success and never brings it back. Restart=always. 3. ledmatrix-wifi-monitor.service used StandardOutput=syslog, which systemd has marked obsolete; it warns and rewrites it to journal on every load. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * perf(memory): size the cache to the board and stop reinstalling deps On a 1GB Pi 3B+ the display process settles around 600MB RSS of 905MB total. When the remaining headroom runs out the failure is not a clean crash: fork() starts returning ENOMEM, so sshd accepts connections and closes them before its banner, timer jobs stop running, and the panel goes dark, while already-resident processes keep serving normally. The board looks healthy from outside and cannot be logged into. Only a power cycle clears it. Three contributing causes: - MemoryCache had a fixed 1000-entry ceiling. Entries are parsed API payloads of tens of KB, so one ceiling cannot serve both a 512MB Zero 2 W and an 8GB Pi 5. Now scaled from MemTotal (150 entries at <=1GB, 1500 at >=8GB), overridable with LEDMATRIX_CACHE_MAX_ENTRIES. - requirements_are_satisfied() returned False for any requirement with extras, so a plugin depending on python-socketio[client] re-ran pip on every single start: ~8s, a network dependency, and a 100-200MB spike at the least convenient moment. During a restart loop it repeats for each restart. Extras are now resolved one level deep against installed metadata, keeping the conservative "anything unverifiable falls through to pip" contract. - ledmatrix.service had no memory ceiling. MemoryMax=85% expressed as a percentage so one unit file suits every board. Note this needs the memory cgroup controller, which Pi firmware disables by default; first_time_install.sh now adds cgroup_enable=memory to cmdline.txt, and the unit file documents how to verify it took effect. first_time_install.sh also enables persistent journald storage (capped at 64M). Default storage is volatile, so every reboot destroys the logs that would explain why the board rebooted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * docs: guidance for 512MB and 1GB boards Documents the memory ceiling on small boards and, more usefully, what running into it actually looks like: sshd accepting connections and closing them before the banner, the web UI still responding normally, clean ping, a dark panel, and a wrong clock after the next boot. None of those read as "out of memory", which makes the failure hard to identify from the symptoms. Cross-referenced from SSH_UNAVAILABLE_AFTER_INSTALL.md, since "I can't SSH in any more" is how most people will first meet this. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix: address review findings on the low-memory work Nine CodeRabbit findings, five in code. **Health state (the one that matters).** The non-dict guard did not cover a dict missing fields the callers index directly, which is the shape actually seen in the wild: a record carrying only circuit_state produced `plugin clock-simple operation failed: 'circuit_state'` about fifty times a minute with the panel frozen. The record is now completed against the defaults per field rather than trusted or discarded wholesale. Per field matters: a first pass rejected any incomplete record outright, which reset a tripped breaker and real failure counts to healthy because one optional field was absent -- an existing test caught it. Values of the wrong type (a counter persisted as a string, an unknown circuit_state) fall back individually, valid neighbours survive, and newer fields the schema has grown since (degraded, degraded_reason) are carried through untouched. **Cache ceiling.** MemoryCache.set() accepted entries without bound between cleanup sweeps, which run every 300s by default, so a burst could take the cache far past max_size -- the unbounded growth the limit exists to stop. Eviction now runs under the same lock on every write, sharing one helper with the periodic sweep so the two cannot drift. **Installer, cgroups.** Only cgroup_enable=memory was checked, so a board carrying that without cgroup_memory=1 reported success and got no change, leaving MemoryMax= inert. Each parameter is now checked and appended independently; verified against all four combinations, single line preserved. **Installer, journald.** Persistence was inferred from /var/log/journal being non-empty, which proves neither Storage=persistent nor a size cap -- the directory survives a switch back to volatile. The effective configuration is read instead (systemd-analyze cat-config, falling back to the conf files), and an explicitly configured SystemMaxUse is preserved rather than overwritten. Verified across volatile, persistent-without-cap, persistent-with-user-cap, cap-without-storage, and commented-only configs. **Dependency extras.** _extras_are_satisfied stopped at one level, so a gated dependency that itself requests an extra (requests[socks]) passed on the base distribution's version while the extra's own dependency was missing, and pip was skipped. It now recurses, with a visited (distribution, extras) set so a cycle terminates. Docs: both kernel command-line paths documented (the installer falls back to /boot/cmdline.txt), daemon-reload and restart added after the systemd override example, memory exhaustion added to the SSH summary with its power-cycle-only recovery, and a language on the fenced block for MD040. Tests: five for the health-state repair including the exact wild shape and that record_failure/record_success no longer raise against it, and one for the cache ceiling. Both mutation-checked. Full suite 2927 passed, with the one pre-existing tmpfs failure that also fails on main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW * fix: harden the health-state repair and confirm journald took effect Second review round; all three findings were valid and two were bugs in the repair added last commit. The repair could raise out of itself. An unhashable circuit_state (a list or dict on disk) hit `value in {...}` and raised TypeError -- from the code whose whole job is to stop a malformed record crashing the caller. It now requires a str before the membership test. bool is a subclass of int, so True passed the timestamp check and then compared as 1.0: enough to expire a cooldown the instant the breaker opened, while False would stop the elapsed check firing at all. Timestamps now exclude bool explicitly. The regression test for the original crash was seeded with a record that *contained* circuit_state, so it passed against the old raw-return behaviour too -- the counters are read with .get(), so circuit_state is the only field whose absence used to raise. Reseeded to omit it, and it now fails against raw-return as intended. journald: drop-ins apply in lexical order, so a local file sorting after ledmatrix-persistent.conf still wins and writing ours proves nothing. The effective Storage is re-read afterwards and a warning naming the diagnostic command is printed if persistence is still not active, rather than reporting a success that was not verified. Full suite 2934 passed, same single pre-existing tmpfs failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>	2026-08-19 12:28:22 -04:00
ChuckandClaude Opus 5	0901d044d3	fix(plugins): report updates that completed, not ones that were queued (#460) ... run_scheduled_updates_with_changes() snapshotted plugin_last_update, called run_scheduled_updates(), and diffed the two to answer "whose data just changed". But run_scheduled_updates() only enqueues. The work runs on the update worker and stamps plugin_last_update there, after this method has already returned, so the two snapshots were always identical and the result was always an empty list. The only path that ever worked was the synchronous kill-switch, where update() runs inline. Vegas is the caller. That empty list is what feeds mark_plugin_updated(), which drops the cached content for a plugin whose data moved -- so a segment kept scrolling whatever it was first built from. It is the failure the coordinator's own comments describe: last night's live game still drawn as live the next morning. On a live rig: zero update ticks in twenty minutes, with weather, stocks and news all updating on schedule. The worker now records each completed update in a ledger and the call drains it, reporting what has finished since the previous poll rather than what this call enqueued. That costs one tick of latency -- Vegas polls every ~4s -- and is correct whichever side of the queue the work lands on. Failure paths are excluded: they stamp the timestamp too, to space out retries, but no fresh data exists. Verified on the rig it was found on: 0 update ticks before, 208 in twenty-five minutes after, naming real plugins. The behavioural tests here would pass with both production call sites deleted, which mutation testing caught -- they drive the ledger directly. So there is also a structural test asserting the invariant at the source: wherever a successful update stamps plugin_last_update, it must record the completion. Writing it immediately caught that _record_update_failure stamps the same field and must not be included. Mutation-checked: removing either call site, removing both, and dropping the drain's clear are all caught. Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-17 13:28:40 -04:00
ChuckandClaude Opus 5	08265c1135	feat(vegas): let live content keep its place in the ticker (#457) ... * feat(vegas): let live content keep its place in the ticker Live content used to preempt Vegas outright: while any plugin reported live priority the display controller refused to run the ticker at all and showed a full-screen scoreboard instead. Keeping the marquee meant not seeing live scores; seeing live scores meant losing the marquee. Two changes, both off by default. vegas_scroll.live_in_ticker keeps the ticker running through a live game. Three places assumed the takeover and all three now honour it: the controller's gate, the coordinator's per-frame pause, and the rotation switch that would otherwise move current_mode_index underneath a ticker that never yields. And the rotation is no longer a strict round robin. It was one slot per plugin per cycle, so with a dozen plugins enabled a live score came round once a lap and could be minutes old on screen. A plugin can now hold several slots, placed by Smooth Weighted Round-Robin -- the same scheduler the sports plugins already use to rotate their own games. The property that matters is that repeats are spread through the cycle rather than clumped: three in a row and then silence would be worse than no boost at all. Weight comes from the plugin first, via a new optional get_vegas_priority_weight(), then from the core: live content earns live_weight, everything else 1. So existing plugins gain the behaviour without changes, and the hook exists for the one thing the core cannot work out -- the core can see that a game is live but not whose, so only the plugin can say a favorite is playing. Documented in ADVANCED_FEATURES (worked example, why weights are per plugin not per game, and that frequency is not freshness), CONFIG_REFERENCE, PLUGIN_API_REFERENCE, and the config template. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * fix(vegas): carry the new keys through config, and correct two docs Three findings from CodeRabbit, all valid. to_dict() and update() enumerate keys explicitly and had not learned the three new ones, so get_status() never reported them and a live config change never applied -- turning live_in_ticker on in the web UI would have done nothing until a restart. update() clamps the weights exactly as from_config does. The vegas_scroll key count in ADVANCED_FEATURES said 29; the template has 30. My arithmetic, not the reviewer's. The third was a documentation error rather than a code one, and I have fixed it the other way round. The docs claimed a raising get_vegas_priority_weight() is treated as weight 1. The code instead falls through to the core's own live-content check, and that is the better behaviour: the hook is only how a plugin asks for *more* than live_weight, and has_live_priority/has_live_content are separate methods guarded separately, so a plugin with a broken weight calculation should lose the favorite distinction and keep the live boost. Said so in the code, the base-plugin docstring and the API reference. The test fake now fails in each place independently, because the two failures mean different things: a broken hook still earns live_weight, a plugin that cannot say whether it is live has nothing to fall back on and weighs 1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ui/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * fix(vegas): stop the heaviest plugin doubling across the cycle seam Smooth Weighted Round-Robin spaces repeats well within a pass, but it schedules the heaviest item first and usually last as well. The strip loops, so those two are neighbours: the marquee showed the same plugin twice running at exactly the one join a within-cycle check cannot see. Observed on a live rig at 28 slots -- gaps of 6, 7, 7, 7 and then 1. Rotating the list does not fix it. Rotation preserves the cyclic order exactly, so it moves where the seam is drawn rather than the adjacency itself; the trailing entry has to be swapped with one from the middle. The first version swapped with the first slot that merely fitted, which undid the spacing this exists to protect -- it moved a repeat from a gap of 7 into a gap of 2, more clumped than the seam had ever been. It now picks the candidate furthest from any other appearance, so the repeat lands in the widest gap. Left alone when no candidate exists. A plugin holding most of the slots has to neighbour itself, and scheduling it is better than refusing to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * fix(vegas): stop the seam repair creating the duplicate it removes Swapping the trailing repeat with a middle slot moves two elements, and the candidate filter only guarded one of them. It checked the neighbours `repeated` would acquire at j, but not what the displaced element would sit beside at the end -- so ['a','b','c','d','x','y','x','a'] came back as [...,'x','x'], the seam duplicate traded for a fresh one. Reported by CodeRabbit with that exact case. Adding the missing condition fixed it and immediately broke something else: schedule[j] is schedule[-2] when j is the second-to-last slot, so that candidate was always excluded, and ['a','b','c','a'] lost the only repair it has. The same class of mistake twice, from reasoning about which neighbours two moved elements end up with. So it no longer reasons. It performs each candidate swap, counts the cyclic duplicates in the result, and keeps the best one that has none -- preferring whichever leaves the boosted plugin most evenly spread. When no such swap exists the schedule is returned untouched, which is the unavoidable case: a plugin holding most of the slots has to neighbour itself. Fuzzed across 6,956 seam schedules: none made worse, none lost an entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-13 17:02:36 -04:00
ChuckandClaude Opus 5	f887063434	test(harness): flag a mode that draws nothing without reporting it (#447) ... The controller skips a mode whose display() returns False and treats anything else -- including None -- as "content was shown". A mode that draws nothing and does not return False is therefore never skipped, and because a mode switch clears the panel first, it sits on a blank screen for its whole display duration. Two sports plugins shipped exactly that. The harness rendered those modes and passed them, because it called display() and discarded the result. Capture it, and warn when a render produced no lit pixels while claiming content. Warn-only by default, and deliberately so: a scroll mode's first frame is legitimately its blank scroll-in buffer, which is 42 of these on the F1 scoreboard alone. Plugins whose modes are known to draw on their fixture data can opt into failing via harness.json {"empty_check": "strict"}, matching how the fill check is staged. Worth being clear about the limit: this only sees what the fixtures render. It would not have caught the sports bug, whose fixture seeds games so the empty path never renders -- that needs the source-level gate in the plugins repo. What it does catch is the same mistake in any plugin whose empty state the harness does happen to reach, which is coverage there was none of before. Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-10 08:17:57 -04:00
ChuckandClaude Fable 5	ee59caa577	Follow-ups from #441: secret-helper migration, ten more bug fixes, and coverage for every remaining untested module (#444) ... * refactor(web): use canonical secret helpers in api_v3; make ConfigManager secret strip/merge array-aware api_v3.py carried three inline nested copies of find_secret_fields/ separate_secrets (main-config save, plugin-config save, plugin-config reset). They drifted from each other (one lacked isinstance guards) and none supported the canonical module's array-item secrets (accounts[].token). All three endpoints now import from src/web_interface/secret_helpers. Adopting the canonical behavior makes array-item secrets reachable, and their parallel-placeholder shape ([{'token': ...}, {}] alongside the regular list) was not survivable by ConfigManager's round-trip: _strip_secrets_recursive dropped the whole key (losing the regular fields from config.json) and _deep_merge replaced the regular list wholesale on load. Both are now array-aware: - strip removes the secret fields from each item and ALWAYS keeps the list so indices survive for merge-on-load; whole-key secrets (scalar lists, shape mismatches) still drop the key entirely — never leak. - merge folds each secrets item into the config item at the same index, skipping {} placeholders. The regular list's length is authoritative in both directions: a user deleting an array item never has it resurrected from a stale secrets entry (extras warn and are ignored). api_v3's own deep_merge intentionally still replaces lists wholesale — form posts carry complete arrays and index-merging would resurrect deleted items; a comment now documents that. Tests: the parity guard flips from 'exactly 3 inline copies' to 'zero, and the canonical import must exist'; TestArraySecretStripAndMerge covers the new strip/merge semantics incl. length-mismatch contracts; new test_api_v3_secret_roundtrip.py drives all three endpoints through a Flask client with a REAL ConfigManager+SchemaManager over tmp_path, proving secrets land in config_secrets.json, config.json stays clean, and a fresh load merges them back into the right array items. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * fix: repair broken helper paths across display, cache, odds, logging, resolver, repos, config, validator Nine fixes for bugs surfaced while writing coverage for previously untested modules (plus the bool-duration quirk pinned in PR #441): - base_plugin.get_display_duration: exclude bools from both numeric branches — display_duration=True no longer reads as a 1-second slot; it falls through to config, then the 15.0 default. - display_helper: draw_error_message/draw_no_data_message called _draw_centered_text with the wrong arguments and crashed with AttributeError — both now delegate to draw_centered_text. draw_scorebug_layout drew status and clock at the same y, overprinting each other — they now share one combined top line. draw_ticker_layout drew its text starting at x=display_width (fully off-canvas), returning a blank frame every time — now draws at x=0; scroll_speed stays accepted-but-unused and is documented as such. - api_helper.clear_cache guarded on a nonexistent CacheManager.clear() method, silently never clearing anything; it now uses the real surface (clear_cache/delete/list_cache_files) and no-ops safely otherwise. - base_odds_manager._extract_espn_data raised AttributeError when ESPN sent explicit JSON nulls ("homeTeamOdds": null) — every level now null-safes with 'or {}'. format_odds_summary gated on is_odds_available, which deliberately ignores money lines, so ML-only odds formatted as "No odds available" — it now gates only on empty/no_odds data and formats money lines. - logging_config.ContextualFormatter mutated record.msg in place, so a second handler prepended the context prefix twice; it now formats a copy. log_error hardcoded exc_info=True and raised TypeError when the caller passed exc_info — now kwargs.setdefault. - dynamic_team_resolver wrote its "shared" class cache through self, creating instance shadows — the cache was per-instance and every scoreboard refetched rankings. Writes now go through the class. - saved_repositories cleaned URLs with an unanchored .replace('.git','') that mangled URLs merely containing '.git' (my.github.io -> myhub.io); now strips only a trailing suffix. add/remove also roll back the in-memory list when the save fails, so memory always matches disk. - config_helper.merge_configs shallow-copied the base, aliasing every un-overridden nested dict into the result — now deep-copies. - startup_validator.validate_all accumulated errors/warnings across calls — now resets both lists per run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * test: cover the previously untested modules Nine new suites plus an extension, asserting the Phase-1b fixed behavior and pinning the quirks deliberately left alone: - test_logging_config.py: formatters (JSON shape, no record mutation, single prefix through two handlers), PluginLoggerAdapter precedence, setup_logging handler hygiene and LEDMATRIX_DEBUG, log_error exc_info. - test_startup_validator.py: exact messages, error-vs-warning split, accessor split (load_config vs get_config), cache-dir branches with os.access monkeypatched (root can write anything in CI), idempotence, raise_on_errors classification precedence. - test_config_helper.py (full): load/save round trips, dot-notation get/set incl. silent-failure contract, post-fix no-aliasing merge, schema validation branches, the '{id}_config' key pin, default-enabled pin. - test_saved_repositories.py: three load shapes, bare-list rewrite pin, trailing-only .git strip (my.github.io regression), save-failure rollback, type-classification case-sensitivity pin. - test_api_helper.py: rate-limit math, cache-hit short circuit, ESPN URL/key formats, exact User-Agent guard, retry adapter, post-fix clear_cache against the real CacheManager surface, ttl-dropped pin. - test_base_odds_manager.py: cache-key/URL construction, no_odds sentinel round trip, stale-cache fallback, null-safe extraction, ML-only formatting, is_odds_available truth table (ML-blind by contract), config key/attr mismatch pin. - test_dynamic_team_resolver.py: expansion/dedup/slicing, dropped unknown-dynamic names (TOP_ substring hazard pinned), genuinely shared class cache (second instance: zero HTTP), TTL expiry, failure degradation without raising. - test_display_helper.py (full): the fixed error/no-data renders, combined scorebug top line, non-blank ticker with scroll_speed no-op pin, composite upconversion, logo bleed positions, square orientation pin. - test_skin_runtime_cache.py: discovery-cache hit/invalidation semantics (manifest mtime, .py edits pinned as non-invalidating), sys.modules namespacing contract incl. bare-name restore and stdlib shadowing, entry-module execute-once, API minor-version tolerance, skin_matches_target table. - test_sports_capabilities.py (extended): _draw_celebration_layout executed for real (flash window, matrix-dims fallback, highlight alternation, logo-failure isolation), _should_celebrate_for direct, strict duration boundary, score_to_int edges, both-teams-score precedence, expired-coalesce refire, disabled-win baseline preservation, id-less prune. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * test: real schedule/dim coverage for DisplayController; fix two vacuous schedule tests New test_display_controller_schedule.py drives _check_schedule and _check_dim_schedule on a bare controller stub: same-day and midnight-crossing windows with inclusive boundaries, global vs per-day vs legacy-inferred modes (and dim's global-only default — no legacy inference), per-day disabled days, invalid %H:%M fallbacks, unknown timezone -> UTC, dim_brightness default 30, inactive-display short circuit, and the _was_display_active/_was_dimmed transition flags. test_display_controller.py's test_schedule_disabled and test_active_hours patched config_service.get_config — which _check_schedule never reads — so both asserted the init-default value and could not fail. Rewritten on the test_inactive_hours pattern (inject controller.config['schedule'], reset the minute gate, flip the flag to the opposite state first so the assertion has teeth). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * ci: raise coverage floor to 48% Measured 50% with the new suites in place (was 47% baseline when the gate was introduced at 45); floor stays two points under measured. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * fix: address CodeQL alert and review findings - config_manager: the "secrets list longer than config list" warning now interpolates only config-side data (no key name or secrets-derived values), resolving the CodeQL clear-text-logging alert. - base_plugin: validate_config rejects bool display_duration, matching get_display_duration (bool is an int subclass and would otherwise pass as a positive number). - config_helper: merge_configs deep-copies override values in the non-recursive branch so mutating the merged result cannot reach back into override_config. - saved_repositories: saves are atomic (temp file + fsync + os.replace), so a failed write can no longer truncate saved_repositories.json. - tests: regression cases for each fix, plus a pin that whole-item array secrets (key[] + key[].field both marked) strip to empty {} skeletons — no secret values can reach config.json. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-08-07 16:17:11 -04:00
ChuckandClaude	31d607f6b3	fix(backup): make a restored device match the one that was backed up (#439) ... * fix(backup): make a restored device match the one that was backed up Found by wiping a working device and reinstalling from scratch. Every problem here is invisible until you actually do that, which is why a green test suite and eleven hours of uptime had not surfaced any of them. **Four enabled plugins vanished on restore.** Weather, stocks, music and leaderboard have a registry `id` that differs from the `id` in their own manifest: the registry calls them `weather`, everything else calls them `ledmatrix-weather`. Installation already prefers the manifest id for the directory name and warns when the two disagree, so on disk, in config.json and in a backup they are `ledmatrix-weather` -- but nothing resolved that in reverse. Restore asked the store for `ledmatrix-weather` and got "Plugin not found in registry", four times, and the device came back missing four plugins the user had enabled. Registry lookup now falls back to matching `plugin_path`, which already records `plugins/ledmatrix-weather`. Renaming the published ids would have orphaned `plugin_state.json` entries keyed on the old ones. Exact id still wins, so a path that collides with another entry's id cannot shadow it. Against the live registry and a real 28-plugin install this takes unresolvable directories from five to one -- the one being starlark-apps, which is genuinely not in the registry. **Secrets could not be restored at all.** A fresh install left config_secrets.json group-readable but not group-writable, and the web interface -- which is what performs a restore -- does not necessarily run as the owner. Every other file in the backup restored; secrets failed with EACCES. Now group-writable, so the account running the web UI can put them back. **A partial restore reported "Restore had errors" and nothing else.** That is the same message whether the whole thing failed or it quietly dropped your API keys. It now names what was restored, what failed, and which plugins were not reinstalled. **ytm_auth.json was never in the backup.** It sits in config/ beside the three files that are, and is pure device-local auth: losing it silently signs the user out of YouTube Music. Backed up and restored with the wifi config, which it resembles. **Backups were written inside the directory a reinstall deletes.** config/backups/exports is destroyed by the reinstall the user was told to make it before. Exports now go beside the install, falling back to the old path when that is not writable. **The installer reboots without asking in non-interactive mode**, which the README did not mention -- easy to hit when piping the install, and alarming when a device you are installing onto disappears. Documented, with --no-reboot-prompt. Its log also claimed root:ledmatrix while printing a hardcoded group name rather than the one it used. Tests: registry resolution gets its own suite, including the collision case and third-party entries with an empty plugin_path. The existing round-trip test passed throughout this because its fixture plugin has a directory name equal to its id -- the one shape that cannot fail -- so it now carries ytm_auth too. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * ci: run the backup suites test_backup_manager.py existed but was never enrolled, so the tests that should have guarded backup and restore have not run on a pull request. That is part of why the restore bugs in the previous commit reached a device: the suite was there, it just was not watching. Adds it alongside the new registry-resolution tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * fix(backup): restore must not depend on owning the file it replaces Follow-up from testing the previous commit on real hardware, where the secrets fix turned out to be both too narrow and slightly wrong. Too narrow: config.json, wifi_config.json and ytm_auth.json are installed root-owned and group-readable exactly like the secrets file, so all four were unrestorable by the web service, not just one. `shutil.copy2` opens the destination for writing, which needs permission on the *existing file*; the web user could create files in that directory all day and still not replace them. Slightly wrong: the previous commit loosened the secrets file to group-writable. That was treating the symptom. The real error was deciding ownership from `ledmatrix.service` -- the display service, which runs as root and only ever *reads* secrets -- when the account that *writes* them is the web interface, which deliberately does not run as root. Ownership now follows the web service's user and the mode stays 640. `_copy_file` writes a temporary file alongside the target and renames over it. That needs only directory permission, so a restore no longer cares who owns the destination, and it is atomic: a crash mid-restore can no longer leave a half-written config. The destination's mode is carried across so restoring secrets does not widen them to the umask, and its owner is carried across too when the OS allows it -- only root can hand a file to another user, so a restore run by the web service keeps its own ownership rather than pretending to preserve root's. Verified on a device with all four config files set root-owned 640 and unwritable by the web user: before, every one failed with EACCES; after, the restore reports success with no errors and all four sections restored, mode still 640, root still able to read them and the web service still able to write them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * fix(backup): address CodeRabbit and CodeQL findings on PR #439 - first_time_install.sh: verify chown/chmod succeed and the final owner/group/mode on config_secrets.json before reporting success; exit with a clear error otherwise instead of swallowing failures. - api_v3.py: replace the predictable .writetest probe with an exclusive NamedTemporaryFile to avoid a race with concurrent resolvers; log the preferred/fallback export path and OSError when falling back to the reinstall-deleted directory. - api_v3.py: mark a restore as failed when plugin reinstalls fail, even if file restoration itself succeeded, so the endpoint no longer reports HTTP 200 success on a partial restore. - api_v3.py: stringify plugin IDs before joining them into the error message so a malformed backup's non-string plugin_id can't raise a TypeError and mask the detailed response. - backup_manager.py / api_v3.py: stop putting raw exception text (originating from a user-controlled backup file) into restore results returned to the client; log full details server-side instead. Addresses the CodeQL "stack trace information exposure" alert. - test coverage: add a test for get_plugin_info() resolving a manifest id, and assert the disabled restore_wifi path also skips and omits ytm_auth.json. Co-authored-by: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-07 13:02:59 -04:00
ChuckandClaude Fable 5	d6c5f97c13	Test suite overhaul + fixes for the three bugs it uncovered (#441) ... * ci: run the whole test tree and make the plugin-safety job assert something real The unit-tests CI job ran an explicit 24-file allowlist that had rotted: 63 of 90 test files (display, vegas, store manager, web API, web_interface) never ran on a PR. The job now runs all of test/ (minus test/plugins, which the plugin-safety job owns) so new test files are enrolled by default and any exclusion needs a visible, commented --ignore. The plugin-safety job was a green no-op: plugins/ is empty in CI, so every test skipped with 'Manifest not found'. It now renders a bundled deterministic fixture plugin (test/fixtures/plugins/ci-fixture-plugin, golden images included for all 8 default sizes) via LEDMATRIX_PLUGINS_DIR, and sets LEDMATRIX_REQUIRE_PLUGINS=1 so discovering zero plugins fails loudly instead of skipping green. The per-plugin suites document that they target dev machines with real plugins installed. Coverage is now measured and enforced in exactly one place — the CI unit-tests step (--cov=src --cov=web_interface --cov-fail-under=45, from a measured 47% baseline). pytest.ini previously declared --cov-fail-under=30 but CI always passed --no-cov, so the gate had never run anywhere; local pytest is now coverage-free and fast. Enabling the 63 unenrolled files surfaced three cases of test rot, fixed here: test_display_controller_vegas_tick.py could not collect without the hardware rgbmatrix module (now uses the emulator convention), the state-reconciliation unrecoverable-cache tests broke when production added the is_plugin_uninstalled tombstone check (bare Mock returned truthy), and test_get_system_status assumed the optional psutil dependency (now installed via requirements-test.txt and guarded by importorskip). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * test: replace can't-fail tests with real assertions test_font_manager.py was 5 of 6 tests shaped as 'try: call(); assert True / except: assert True' — running in CI while unable to fail on any regression. Rewritten against the real FontManager API and the bundled assets/fonts: returned font types, cache-hit identity, distinct entries per size, default-font fallback for unknown families and corrupt files (recorded in failed_loads), BDF native-size reading, text measurement, and cache lifecycle. test_display_manager.py's test_draw_text ended in 'assert True'; it now renders onto a known-black canvas and asserts pixels were actually lit — which required un-breaking the fixture's freetype MagicMock so draw_text's isinstance check doesn't silently swallow the draw. test_display_controller.py carried a permanently-skipped test whose skip reason already declared it redundant; deleted. Both display test files now set EMULATOR=true before importing display_manager (the same convention as test_display_dirty_tracking.py) so they collect standalone instead of depending on which test module imports display_manager first. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * test: cover the untested fragile logic (compatibility gate, secrets, config merges, durations, skin cards) New unit tests for pure or filesystem-only logic that previously had zero direct coverage: - test_compatibility.py: the semver install gate (parse_semver suffix handling, every range operator, TRUSTWORTHY_FLOOR behavior for cores reporting untrustworthy versions, 'more restrictive wins', and the malformed-manifest shapes that used to raise). - test/web_interface/test_secret_helpers.py: the canonical x-secret helpers — find/separate/mask/remove, array-item secrets, no input mutation, and a separate->recombine round-trip. - test/web_interface/test_api_v3_helpers.py: the module-level helpers behind the plugin config save endpoint (_is_plugin_update_available, _coerce_to_bool including the int==1 quirk, deep_merge including its shared-subtree shallowness, _parse_form_value, dotted-key-aware _get_schema_property/_set_nested_value). - test_base_plugin_duration.py: get_display_duration's full coercion ladder (instance attr -> config -> 15.0), including the bool-is-int quirk where display_duration=True means one second. - test_config_manager_secrets.py: the secrets round-trip — deep-merge on load, strip on save, group pruning, the load fast path — and two characterized sharp edges marked SUSPECTED BUG: an unreadable secrets file at save time writes secrets into config.json in plaintext, and a same-mtime-same-size content swap is served stale. - test_schema_manager_merge.py: merge_with_defaults branch behavior (None replacement vs falsey preservation, dict-vs-scalar mismatches, arrays replaced wholesale, defaults never mutated). - test_skin_system.py (extended): render_skin_card shares _render_game's 3-strike counter but never resets it on success — the asymmetry is pinned in both directions, along with card fallthrough and the disable interaction between the two paths. Suspected bugs are characterized, not fixed — each carries a comment so a future behavior change is deliberate rather than accidental. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * test: add drift guards for cross-file contracts Three guard suites that pin contracts spanning multiple files, where one side changing unilaterally breaks the other silently: - test_version_comparison_consistency.py: the repo's four version comparators (compatibility.parse_semver, api_v3's packaging-based _is_plugin_update_available, store_manager update_plugin's raw string equality, skin_runtime._major) answer differently on the same inputs. A table pins each one's verdict; update_plugin is driven through its real code path to show the SUSPECTED BUGs: 'v1.2.0' vs '1.2.0' triggers a full reinstall the UI calls unnecessary, and a locally-ahead plugin gets downgraded. A pairwise-ordering check keeps parse_semver agreeing with packaging on plain X.Y.Z. - test/web_interface/test_secret_separation_parity.py: api_v3.py carries three inline copies of find_secret_fields/separate_secrets that lack the canonical module's array-item support. The copy count is asserted exact (it may only go down; new copies must import src/web_interface/secret_helpers), the missing-array-support gap is asserted so it can't grow silently, and the canonical behavior that migration will adopt is documented executably. - test_discovery_path_contract.py: the three 'where is plugin X' resolvers (PluginManager discovery, StoreManager._find_plugin_path, SchemaManager.get_schema_path) agree on the configured directory, and their divergent fallback chains are characterized. Also pins the .standalone-backup- naming contract shared by store rollback and discovery, and _resolve_skin_target's path-traversal rejection. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * test: address review feedback — fixture lifecycle, test names, ClassVar - ci-fixture-plugin: call display_manager.clear() before rendering (per plugin guidelines — the fixture should model a well-behaved plugin), add a class docstring, and document why Pillow is deliberately not pinned in its requirements.txt (core dependency; harness installs nothing). - Rename two tests whose names contradicted their assertions: test_unparseable_core_version_is_compatible -> test_unparseable_core_with_high_floor_is_blocked, and test_unreadable_secrets_file... -> test_corrupt_secrets_file... - Annotate TestGetSchemaProperty.SCHEMA as ClassVar (RUF012). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * ci: allow manual test.yml runs via workflow_dispatch Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * fix: unify version comparison, refuse secret-leaking saves, reset skin strikes on card success Fixes the three suspected bugs this PR's characterization tests pinned, flipping those tests to assert the corrected behavior: - plugins/store: ONE shared update comparator. New compatibility.is_update_available() (PEP 440 via packaging) is now used by both the web UI's update badge (api_v3._is_plugin_update_available is a thin alias) and store_manager.update_plugin's reinstall decision. Previously update_plugin used raw string equality: 'v1.2.0' vs '1.2.0' triggered a full reinstall the UI called unnecessary, and a locally- ahead plugin (2.0.0 installed, registry 1.9.0) was silently DOWNGRADED. Now equivalent spellings skip the reinstall and locally-ahead versions are never downgraded; unparseable versions still reconcile by reinstalling from the registry. - config: save_config and save_config_atomic now refuse (ConfigError) when config_secrets.json exists but cannot be loaded. Both previously proceeded without stripping, writing the merged secrets into config.json in plaintext. The shared _load_secrets_for_save() helper raises with an actionable message instead; a missing secrets file is still fine (nothing to strip), and _migrate_config's catch-all keeps boot resilient. - skins: render_skin_card resets _skin_failures on both success paths (vegas card returned, or mode renderer handled), mirroring _render_game. Transient card failures no longer accumulate across a session until they permanently disable a working skin. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh * fix: harden shared comparator edges from review - is_update_available: reject truthy non-string versions (a malformed manifest can carry a number; packaging raises TypeError on those) by surfacing the mismatch instead of raising. - store_manager.update_plugin: drop the truthiness gate around the comparator so a missing version on either side follows the shared 'no update' verdict, keeping the store consistent with the UI badge; a missing manifest still uses the reinstall recovery path. - config_manager._load_secrets_for_save: catch only expected read/parse failures (OSError/ValueError/RecursionError) so implementation bugs propagate as themselves, and log with traceback. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NohXi78cwsAKtN1sCfxjUh --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-08-07 10:17:30 -04:00
ChuckandClaude Fable 5	d9683e28be	Codebase audit: fix shipping bugs, remove verified-dead code, repair doc drift, add regression guards (#438) ... * fix(web): implement delete_cached so the font catalog cache actually invalidates api_v3.py's font upload/delete handlers import delete_cached from web_interface.cache, but the function was never defined. The surrounding except ImportError silently swallowed the failure, so the fonts_catalog cache entry survived uploads/deletes and newly uploaded fonts did not appear until the TTL expired or the service restarted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * fix(web): remove dead weather/stocks partial routes that returned 500 The partial dispatcher still routed 'weather' and 'stocks' to loaders rendering v3/partials/weather.html and stocks.html — templates that no longer exist since weather and stocks became store plugins. Requesting either partial raised TemplateNotFound, which the catch-all turned into a 500. No template or JS references these partials (the only 'weather' hit in the front end is a plugin-store category filter option), so the branches and both loader functions are removed; unknown partials now fall through to the existing 404 handler. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * fix(deps): align contradictory psutil/Flask-Limiter/freetype-py pins requirements.txt's optional-install comment recommended psutil>=5.9,<6.0 while web_interface/requirements.txt hard-requires >=6.0,<7.0 — anyone following the comment ends up with an unsatisfiable pair. The comment now recommends the same range the web interface requires (all psutil APIs used — Process, boot_time, cpu_percent, disk_usage, virtual_memory — are stable in 6.x). Flask-Limiter gains the same <4.0 cap in both files and freetype-py the same >=2.5.1 floor. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * fix(config): add template keys the code already reads display.hardware gains pixel_mapper_config, row_address_type, multiplexing and panel_type (read at display_manager.py with these exact fallbacks — users on non-standard panels previously had no way to discover them from the template). vegas_scroll gains frame_based_scrolling and scroll_delay, the only two of its 27 keys the template omitted (read in src/vegas_mode/config.py). plugin_system gains development_mode, which the web UI reads and writes but the template never declared. Every added value is byte-identical to the code-side .get() fallback, so ConfigManager._migrate_config() merging these keys into existing user configs cannot change behavior on any installed device. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * fix(scripts): repair broken sys.path setup in utility scripts clear_cache.py and download_nba_logos.py pointed sys.path at a 'src' directory relative to the script's own folder (scripts/utils/src and scripts/src — neither exists), so both crashed on import; they now insert the project root and import via the src package like the other scripts. debug_web_manual.py resolved 'project root' to scripts/debug/ instead of two levels up. fix_nhl_cache.sh is removed: it used Python docstring syntax in a bash script and invoked clear_nhl_cache.py, which does not exist anywhere in the repo — it cannot ever have worked in its current location. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * docs: correct stale file:line references and the loader-fallback contradiction CLAUDE.md and .cursorrules disagreed about plugin-directory fallback behavior; the code (SchemaManager.get_schema_path) probes plugins/ BEFORE plugin-repos/, and the main discovery path has no fallback at all — both files now describe the real behavior, preferring symbol names over line numbers so the references rot slower. REST_API_REFERENCE.md pointed at app.py:144/:607 for mounts that live at :199/:799 and counted 92 routes where there are 94. PLUGIN_ARCHITECTURE_SPEC.md's historical banner gains a note that its example imports (src/plugin_system/base_classes/*_plugin.py) never shipped — the real base classes are src.base_classes.sports.SportsCore and src.base_classes.hockey.Hockey. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * docs: fix broken links, phantom script references, and stale CI description Repairs every broken relative link in active docs (targets renamed or archived long ago: PLUGIN_DEVELOPMENT.md -> PLUGIN_DEVELOPMENT_GUIDE.md, API_REFERENCE.md -> REST_API_REFERENCE.md, PLUGIN_STORE_USER_GUIDE.md -> PLUGIN_STORE_GUIDE.md, plugin_docs/ dir, TROUBLESHOOTING_QUICK_START.md, and MIGRATION_GUIDE's README link that silently resolved to the docs index instead of the project README). Replaces commands invoking scripts that do not exist (scripts/update_stats.py, validate_registry.py, check_updates.py, fix_permissions.sh) with the real tooling, and rewrites HOW_TO_RUN_TESTS.md's CI section, which described a security-audit workflow that was never committed and a pytest workflow 'queued to land' that landed long ago as test.yml. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * docs: complete the docs index and refresh the web interface file tree docs/README.md's own policy says every page must be linked from the index, yet five weren't — including the entire skin system (SKIN_SYSTEM.md, CREATING_SKINS.md), ADAPTIVE_LAYOUT.md, plugin-safety-harness.md and SPORTS_UNIFICATION.md. Each is now listed in the section it belongs to, and PLUGIN_ARCHITECTURE_SPEC.md is marked historical in the index (the doc itself already carries the banner). web_interface/README.md's static/v3 tree showed only app.css/app.js; it now reflects the actual contents. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * chore: remove dead modules confirmed unused in-repo and across all store plugins - src/common/cli.py: imports a 'ledmatrix_common' package that exists nowhere (not in this repo, any requirements file, or the plugin monorepo), so it cannot ever have run; its README section claimed scripts/dev/* used it, which was also untrue. - src/web_interface/logging_config.py: zero callers — the web app uses web_interface/logging_config.py (a different module), and nothing imports the src copy. - handle_errors decorator in src/web_interface/error_handler.py: zero call sites (the module's response helpers stay — they are used). - ConfigManager.get_clock_config(): reads a 'clock' config key that no longer exists anywhere; only caller was its own unit test. Deliberately kept despite zero in-repo callers: DisplayError, src/common/config_helper.py and display_helper.py — all documented as plugin-facing API (docs/PLUGIN_ERROR_HANDLING.md, src/common/README.md), and third-party plugins outside the official monorepo cannot be enumerated. Verified against a fresh clone of ledmatrix-plugins (43 plugins): zero references to any removed symbol. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * chore: remove manager-era NBA test files and one-off debug scripts The four test_nba_*.py files imported nba_managers, leaderboard_manager and odds_manager — top-level modules deleted when sports displays became plugins — inside try/except blocks that swallowed the ImportError, so they passed while exercising nothing. test_nba_data_structure.py and debug_nba_api.py (a diagnostic script living in test/) made live ESPN API calls rather than testing repo code. None were enrolled in CI. scripts/debug/direct_fix_imports.py and check_imports.py were one-shot artifacts that edited/inspected a hardcoded ~/LEDMatrix/web_interface/ app.py to fix an import problem solved long ago; nothing references them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * chore: remove generate_report.py, which aggregates artifacts of CI jobs that do not exist The script's only function is to merge JSON artifacts (bandit/semgrep/pip-audit/safety/gitleaks results) produced by a security-audit workflow that was never committed — .github/workflows/ has no such jobs, so there is nothing for it to aggregate and no way to run it usefully. Its siblings stay: prove_security.py and audit_plugins.py both run standalone (verified), and .codacy.yml stays because the Codacy service (README badge) reads it server-side without a workflow file. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * fix(web): load the three widget scripts store plugins already declare time-picker.js, file-upload-single.js and plugin-file-manager.js register widgets that installed store plugins reference in their config schemas (countdown uses x-widget: time-picker and file-upload-single; of-the-day uses plugin-file-manager), but base.html never included the scripts. plugin_config.html renders such fields as an empty container that polls LEDMatrixWidgets.get(...) on a 50ms loop forever, so those plugin config fields appeared permanently blank. The audit initially flagged these files as dead code; the monorepo cross-check proved the opposite — they were unreachable, not unused. example-color-picker.js (the documented custom-widget example) gains an explicit warning that including it in base.html would shadow the built-in color-picker widget. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * chore: drop the legacy youtube block from the secrets template No code reads a top-level youtube secrets key: the youtube-stats plugin receives its API key namespaced under its own plugin id (declared via x-secret in its config schema), like every other store plugin. The key survives only in state_reconciliation.py's non-plugin-key exclusion set, which stays — existing installs still carry the key in their generated config_secrets.json, and the exclusion prevents it from being misclassified as a plugin config. New installs simply stop being asked for a YouTube API key they have nowhere to use. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * chore(deps): remove packages nothing imports, declare direct imports, move mypy to test deps Removed from requirements.txt: python-socketio, python-engineio, websockets, websocket-client — zero imports anywhere in this repo, and the one store plugin that needs Socket.IO (ledmatrix-music) declares it in its own requirements.txt, which the plugin store installs. Removed the same quartet plus timezonefinder, geopy, google-auth-oauthlib, google-auth-httplib2, google-api-python-client, unidecode, icalevents, python-dateutil, flask-wtf and the werkzeug pin from web_interface/requirements.txt — all leftovers from the deleted built-in weather/calendar/music displays (flask-wtf was doubly dead: app.py explicitly disables CSRF and sets csrf=None). scripts/ install_dependencies_apt.py, which mirrors these lists for the first-time installer, drops the same packages. Added: urllib3 (imported directly in four core modules), jinja2 and markupsafe (imported directly in pages_v3.py) — previously reachable only as transitives. mypy moves from runtime requirements to requirements-test.txt. Verified in a fresh venv: all four requirements files co-install, pip check is clean, the full CI-enrolled suite (907 tests) and a Flask boot smoke pass with the trimmed dependency set. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * refactor: single canonical DateTimeEncoder src/cache_manager.py and src/cache/disk_cache.py each defined an identical DateTimeEncoder (datetime -> ISO-8601). The disk_cache copy is the only one actually used for serialization; cache_manager now re-exports it instead of defining a twin, so the two can never silently diverge. Import compatibility is preserved — from src.cache_manager import DateTimeEncoder still works and is the same class object. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * docs(code): document deliberate duplicates instead of merging them The audit surfaced several near-duplicate implementations that turned out to be either deliberate forks or behaviorally different — merging any of them would risk changing behavior on installed devices, so each now carries an explicit comment stating the relationship: - VisualDisplayManager: headless fork of DisplayManager; header now lists the ~15 mirrored methods and warns that DisplayManager changes must be mirrored. - normalize_abbreviation: LogoDownloader's version (called directly by nine scoreboard plugins) replaces filesystem-unsafe characters; LogoHelper's strips spaces. Logo filenames on existing installs depend on both behaviors staying put. - The two PluginTestBase classes: the shipped one is plugin-author API, the repo's own richer harness lives in test/plugins/ — now cross-referenced. Also verified (no change needed): ConfigManager's backup/rollback methods genuinely delegate to AtomicConfigManager, and SportsCore already delegates _read_bdf_native_size to FontManager. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * docs: add a unified configuration reference There was no single place documenting what lives in config.json — display.* keys were scattered across README sections, vegas_scroll lived in ADVANCED_FEATURES.md, and dim_schedule, display.double_sided, sync.follower_position, plugin_system.development_mode and the four newly-templated hardware keys were documented nowhere. CONFIG_REFERENCE.md now lists every template key plus the code-read-only keys, each with type, default, and the code location that reads it, and explains the secrets file's plugin-id namespacing. Linked from the docs index. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * docs: bring the README's feature tour into the plugin era The Core Features section still presented clock/weather/sports/stocks/ music displays as built into the project, when all of them are store plugins installed from the ledmatrix-plugins monorepo — only starlark-apps and web-ui-info ship in this repo. The intro now says so (the showcase itself is unchanged; those are real displays available in the store). The display_durations reference drops its built-in-calendar example in favor of plugin-id keys, and the Configuration section links the new CONFIG_REFERENCE.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * docs: archive the custom-icons status report, cross-link config docs, document assets/ PLUGIN_CUSTOM_ICONS_FEATURE.md was a 'What Was Implemented' status report duplicating the actual guide (PLUGIN_CUSTOM_ICONS.md) — moved to docs/archive/ per the docs index's own policy. The overlapping plugin-config docs keep their content but PLUGIN_CONFIG_ARCHITECTURE.md now states up front which doc is canonical for which purpose. assets/README.md is new and load-bearing: assets/stocks, weather, news_logos and broadcast_logos have zero references in this repo's code, which makes them look deletable — but store plugins (ledmatrix-stocks, ledmatrix-weather, news, odds-ticker) resolve those exact paths at runtime against the install directory. The README records that evidence so a future cleanup doesn't break installed plugins. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * test: add regression guards for the bug classes fixed in this PR Three lightweight static checks, all enrolled in CI's unit-test allowlist along with the new web-cache test: - test_template_targets.py: every literal render_template() target must exist (would have caught the weather/stocks partial 500s at commit time). - test_widget_scripts.py: every widget JS file must be script-included in base.html or explicitly allowlisted with a reason (would have caught the unloaded time-picker/file-upload-single/plugin-file-manager widgets), and allowlisted files must NOT be included (prevents the example widget from shadowing the real color-picker). - test_doc_links.py: relative markdown links in active docs must resolve (docs/archive/ exempt). Each guard was verified to fail against the pre-PR tree and pass now. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * fix(deps): restore the werkzeug version floor Commit 1ec22db removed the werkzeug>=3.1.6,<4.0.0 pin along with the genuinely-unused packages, but this one was a version floor on Flask's transitive dependency, not a phantom: Flask 3.1.3 itself only requires werkzeug>=3.1.0, so dropping the pin let fresh installs resolve 3.1.0-3.1.5. Restored with a comment explaining why it exists. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * fix(web): route pixel_mapper_config into display.hardware; guard time-picker registration pixel_mapper_config was the only display.hardware key absent from both the display_fields detection allowlist and the hardware write loop in the settings save path. No form posts it today, but if one ever did the key would fall through to the generic handler and land at the TOP level of config.json — where state_reconciliation would mistake it for a missing plugin id and loop auto-repair attempts (the failure class the 'github'/'youtube' exclusion comment documents). It now round-trips into display.hardware like its siblings. time-picker.js gains the same LEDMatrixWidgets-undefined guard its two sibling widgets already have; correct today only via defer ordering. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * chore: align installer leftovers with the dependency cleanup first_time_install.sh's fallback secrets heredoc (used only when the template is missing) still wrote the legacy youtube block — now matches the template (github only). install_dependencies_apt.py drops the IMPORT_NAME_MAP entries for packages no longer in its install lists and a stale google-api reference in a docstring. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * fix: address CodeRabbit review findings Verified each finding against the code; fixes for the valid ones: - install_dependencies_apt.py: the installer listed 'freetype', but the declared dependency is freetype-py — an apt miss would pip-install the wrong PyPI package. Now installs freetype-py with an import-name mapping (pre-existing bug, surfaced by the review). - api_v3.py: pixel_mapper_config is validated as a string before being saved to display.hardware (JSON callers could previously store an object/list the matrix library can't use). - .cursorrules: the Plugin Loading Process and File Organization sections still said discovery scans plugins/ — now consistent with the corrected overview (configured directory, default plugin-repos/). - README.md: removed the stale '(except the core calendar)' claim — no core calendar exists in src/ — and qualified the plugin inventory (official plugins in the monorepo; third-party from their own repos). - CONFIG_REFERENCE.md: hardware_mapping now shows the code fallback (adafruit-hat-pwm) alongside the template value. - PLUGIN_REGISTRY_SETUP_GUIDE.md: check_plugin.py takes --plugin, not a positional id. - scripts/fix_perms/fix_*.sh: exec bits set so the documented 'sudo ./...' invocations work. - Guard tests hardened: template guard now catches multi-line render_template() calls; widget guard parses actual <script> src values and fails if the widgets dir goes missing; type hints and docstrings added per repo coding guidelines. Skipped with reasons (noted on the PR): limit_refresh_rate_hz 100-vs-90 is documented as intentional in CONFIG_REFERENCE.md; the psutil comment already names the enforcing manifest; docs/archive/ findings are out of scope per the docs policy (archive may rot). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * chore: remove Cursor IDE tooling, consolidate its guidance into CLAUDE.md The maintainer no longer uses Cursor. .cursorrules, .cursorignore and the .cursor/ tree (rules, plugin templates, a parallel 751-line plugins guide) are removed; measurement showed near-zero literal overlap risk — the canonical content already lives in docs/. Unique guidance worth keeping moved before deletion: - CLAUDE.md gains the dev workflow (dev_plugin_setup.sh, dev_server.py, run.py -e, check_plugin.py), the plugin-secrets namespacing contract, and the no-draw_image()/paste-onto-PIL pitfall. - PLUGIN_DEVELOPMENT_GUIDE.md absorbs the plugin version-management rules (pre-push hook install, SKIP_TAG, version resolution order) that its own text previously linked out to .cursorrules for. - The one completed plan doc (.cursor/plans/) is archived to docs/archive/ per the docs policy rather than deleted. One of the deleted rule files (sports-managers.mdc) targeted src/*_managers.py globs that have matched nothing since the plugin migration. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * chore: second-pass cleanup — dead installer branch, broken script, orphaned JS, misfiled test deps - first_time_install.sh: removed the pip fallback branch that installed from requirements_web_v2.txt — a file that has not existed since the v2 web interface was removed (the branch always printed its own 'not found; skipping' warning). - scripts/remove_plugin_backups.sh deleted: its PROJECT_ROOT resolved to the repo's PARENT directory, and its verify_submodules() checks for plugin submodules from an era before plugins moved to the store — it could never have worked from its current location. - plugins_manager.js: removed three functions with zero call sites anywhere (addKeyValuePair, formatCommit, togglePasswordVisibility) — verified against all templates, all JS, and the dynamic window[name] dispatch sites, which resolve widget-registry keys only. Also replaced base.html's misleading 'Legacy ... during migration' label: the file is deliberately loaded last and provides the LIVE implementations of seven window.* plugin actions that shadow same-named definitions in app.js/app-shell.js. - pytest/pytest-cov/pytest-mock moved from runtime requirements.txt to requirements-test.txt (CI already installs both files; the installer's line-by-line loop simply installs three fewer packages on devices; no store plugin declares pytest). HOW_TO_RUN_TESTS.md updated. - scripts/add_defaults_to_schemas.py and analyze_plugin_schemas.py scanned the empty legacy plugins/ dir — now scan plugin-repos/. Verified: fresh venv installs all four requirements files with pip check clean and pytest available; bash -n on the installer; node --check on the JS; widget/cache guard tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * docs: correct semantically stale content across the user and developer guides A second-pass content audit checked the guides' substantive claims against the code (the first pass only fixed mechanical drift). Fixes: - GETTING_STARTED: described booting a prebuilt SD image and seeing default clock/weather plugins — neither exists. Now documents the real install (Pi OS Lite + one-shot installer / first_time_install.sh) and that displays come from the Plugin Store. Duration and ordering instructions moved to the Rotation tab where the controls actually live. - WEB_INTERFACE_GUIDE: three whole tabs were undocumented (Rotation, Backup & Restore, Tools) and the Display tab's Vegas Scroll section was unmentioned. Fonts overrides are per display element (not per plugin); Logs has an Auto-scroll checkbox (not a Pause button); the aspirational keyboard-shortcut list and no-JS claim removed. - TROUBLESHOOTING: the hand-written service-file template (wrong user, wrong ExecStart, dropped the autostart gate) replaced with the real systemd/ units + install scripts; recovery steps no longer copy placeholder units verbatim; WiFi curl endpoint corrected to /api/v3/; cache-clearing advice now targets the real cache locations. - ADVANCED_FEATURES: removed a false claim that CacheManager has no delete(); fixed two example snippets that raise TypeError (BackgroundDataService and get_config_file_mode signatures); fixed cache paths, a 5-minute TTL that is actually 1 hour, and the vegas table now links the complete 26-key reference. - EMULATOR_SETUP_GUIDE: documented run.py flags that don't exist (--plugin/--test-plugins) removed in favor of dev_server.py and check_plugin.py; shipped emulator config values corrected (browser adapter default on :8888, not pygame). - PLUGIN_QUICK_REFERENCE: drag-and-drop reordering is shipped, not 'not yet supported'; discovery-fallback and registry-repo claims corrected. PLUGIN_API_REFERENCE: get_vegas_segment_width returns panels, not pixels. CONTRIBUTING: the repo uses flake8/mypy/bandit pre-commit hooks, not black/ruff, and tests need requirements-test.txt. - SKIN_SYSTEM/DEVELOPER_QUICK_REFERENCE: stale module paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr * fix: raise the two new dependency floors past their CVEs, silence a deliberate re-export All three of these were introduced by this PR, which is what makes them worth fixing here rather than deferring. `urllib3` and `jinja2` were added to the requirements so that direct imports stop relying on transitives — right call, but both floors were set to the version that introduced the API rather than a version that is safe to install. `urllib3>=1.26.0` sits below roughly ten CVEs including a decompression-bomb safeguard bypass, and `jinja2>=3.1.0` below five including two sandbox breakouts. Raised to 2.7.0 and 3.1.6, which is what a working device already runs, so no install is disturbed. The comments now say the floor is a security floor, since the next person to read "imported directly" would otherwise reasonably lower it again. This is the same reasoning the PR already applied to werkzeug; these two just missed it. The `DateTimeEncoder` import in cache_manager is unused on purpose — the canonical class moved to src.cache.disk_cache and this re-export keeps the documented import path working. flake8 cannot see intent, so it gets an explicit `# noqa: F401` rather than being removed and quietly breaking anything importing it from here. Verified the re-export still resolves to the same object and still serialises datetimes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * fix: address CodeRabbit re-review — installer robustness and doc lint - install_dependencies_apt.py: an import-only check let Debian Bookworm's python3-freetype 2.3.0 satisfy the freetype-py>=2.5.1 pin. check_package_installed() now verifies the installed freetype-py version, and an apt install that lands below the minimum falls through to pip instead of counting as success. - first_time_install.sh: the .web_deps_installed marker was created even when the smart installer failed, so re-runs skipped installation with dependencies missing. The marker is now created only on success. - CONTRIBUTING.md: document installing the pre-commit CLI before 'pre-commit install' (the requirements files don't provide it). - Doc lint: fence language on the on-demand cache example (MD040), blockquote continuation in GETTING_STARTED (MD028), and the suppress_adapter_load_errors key removed from the emulator debug example to match the options table. Skipped one finding with reason (noted on the PR): the per-plugin display_duration field in PLUGIN_QUICK_REFERENCE's example is not obsolete — BasePlugin.get_display_duration() reads it and PLUGIN_CONFIG_CORE_PROPERTIES.md documents it as a core property; display.display_durations is a per-mode override, not a replacement. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SXb4mKcAkVaxkeTb3YnAdr --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-08-06 14:04:23 -04:00
ChuckandClaude Opus 5	2af41c561b	fix(plugins): make update scheduling atomic so update() cannot run twice at once (#437) ... * fix(plugins): make update scheduling atomic so update() cannot run twice at once Closes #401. `run_scheduled_updates()` decided whether to update a plugin with a check-then-act sequence: `can_execute()` and `set_state(RUNNING)` were separate calls with nothing between them, so two scheduler threads could both observe ENABLED and both go on to call the same plugin's `update()`. `update_all_plugins()` had the identical pattern. Two schedulers really do run at once. The render loop calls `_tick_plugin_updates()`, and Vegas mode fires its own `vegas-plugin-tick` daemon thread that is never joined when `VegasModeCoordinator.play()` returns — a slow `update()` still in flight overlaps the next tick from the main loop. A plugin running `update()` twice concurrently is unsafe unless it happens to be reentrant; shared mutable state, a non-thread-safe HTTP session or cache all break. The async path was already covered by the `_pending_lock` dedup in `_enqueue_update`, so the live exposure was the synchronous kill-switch path and `update_all_plugins()`. Both now claim the plugin through `_reserve_for_update()`, which holds one lock across the eligibility check, the due-time check and the RUNNING transition — and nothing more. Holding it across `execute_update()` would serialize slow plugins behind each other and reintroduce the render stall the async worker exists to avoid. The due-time check moved inside the lock deliberately. Left outside, a thread that had already decided "due" could claim the plugin the instant the winner finished, running `update()` twice within one interval. Two supporting changes fall out of it: - `_enqueue_update()` no longer sets RUNNING (the reservation did), and hands the reservation back if the pending-dedup ever fires. Otherwise a reserved-but-unqueued plugin would sit in RUNNING with nothing left to release it, and `can_execute()` would refuse it forever. - `_finish()` now clears the pending entry *before* flipping the state back to ENABLED. The old order left a window where a scheduler saw ENABLED, reserved the plugin, then had its enqueue silently dropped by the dedup — harmless as a missed tick before, a stuck plugin once a reservation is involved. Regression suite added and enrolled in CI, along with test_async_plugin_updates.py which was not previously run there. The overlap tests delay `can_execute()` to hold every thread inside the check-then-act gap: the real window is a couple of bytecodes wide, so a plain hammering test passes against the unfixed scheduler and proves nothing. With that delay the suite reports `update() ran 8x concurrently` on both affected paths before the fix, and passes after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * test: fix a race in the reservation suite's own wait loops `test_async_path_never_overlaps` failed in CI with "update() ran 0x concurrently" — the test's bug, not the scheduler's. It polled `plugin._active` to wait for the update to finish, but before the worker picks the item up nothing is active yet, so the loop fell straight through and asserted on a plugin that had never run. Both async waits now key on `update_calls >= 1` as well, so they wait for an update to have started *and* finished. The stranded-state test gets the same guard for a second reason: ENABLED is also the starting state, so without it that assertion passes vacuously on a plugin that was never scheduled. Verified over 12 consecutive local runs, 12 passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * fix(plugins): roll back the claim when dispatch fails _enqueue_update() reserved the plugin and added it to the pending set, then started the worker and queued the item. Thread.start() raises RuntimeError when the OS refuses a new thread — not hypothetical on a Pi under memory or thread pressure — and nothing is queued at that point to release the plugin. It stayed RUNNING with a stale pending entry, so can_execute() refused it for the rest of the process, and the exception escaped run_scheduled_updates() and skipped every remaining plugin in that tick. That is the same stranded-RUNNING failure the reservation was introduced to prevent, just reached through the dispatch rather than the dedup, so it is handled the same way: discard the pending entry, hand the reservation back, log the cause. Swallowed rather than raised so one plugin failing to queue cannot abort the others' turn. Both new tests fail against the un-rolled-back version — the second on the escaping RuntimeError itself — and pass with it. 74 tests across the reservation, async-update, plugin-system, health, Vegas-adapter and controller-toggle suites still pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WexvwNDtWLVymGVqKD7BGk --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-05 19:38:11 -04:00
ChuckandClaude Opus 5	d305be6089	fix(plugins): core's own config keys no longer flag plugins as degraded (#434) ... Found while sweeping devpi for issues. Nine of 27 installed plugins were reported degraded in the web UI -- including baseball-scoreboard and f1-scoreboard -- for using a documented core feature. The core reads three tuning keys out of each plugin's own config block: vegas_width_pct and vegas_overflow (vegas_mode/plugin_adapter.py) and vegas_max_width_screens (base_plugin.py). No plugin declares them, and 37 of the 42 published config schemas set "additionalProperties": false -- so schema validation reported them as violations. That is not just log noise. _validate_config_schema_soft sets `degraded` in the health tracker, which the web UI surfaces, so a user who tuned a core Vegas setting saw the plugin marked broken. The keys are stripped before validation. Fixing it plugin-side would mean 42 schema edits and 42 version bumps -- 42 store updates for a contract the core owns. Listed explicitly rather than matched on a `vegas_` prefix: vegas_mode is the opposite case, plugin-owned and declared in schemas, and a prefix rule would silently stop validating it. Verified on devpi: degraded went 9 of 27 -> 0 of 27, schema-mismatch warnings 9 -> 0, 22 plugins still load, no tracebacks. 800 core unit tests pass, 8 of them new -- including that a genuine violation is still reported, so the check has not been turned into a no-op, and that the caller's live config dict is never mutated. Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-04 13:46:28 -04:00
ChuckandClaude Opus 5	53af53b4a1	feat(store): evaluate compatible_versions, not just the floor (#433) ... * feat(store): evaluate compatible_versions, not just the floor Closes the gap CodeRabbit surfaced on #427. `compatible_versions` is the canonical compatibility contract -- schema/manifest_schema.json marks it required, all 42 published manifests carry it -- and it is the only field that can express an *upper* bound. `ledmatrix_min_version` is a floor and cannot say "not compatible with 4.x". The gate read only the floor, so a plugin declaring ["2.0.0 - 2.9.9"] would be installed on 3.2.0 regardless of having said it stops at 2.x. check() now evaluates both and the more restrictive wins. The array is a set of alternatives (satisfying any one entry suffices), supporting every form the schema permits: >=, <=, >, <, ~, ^, a bare exact version, and an inclusive "A - B" range, with prerelease/build suffixes tolerated. Refusal still requires evidence. Anything unparseable, absent, or below TRUSTWORTHY_FLOOR resolves to compatible. That last point needed a new strict parser. parse_semver is deliberately lenient -- it strips non-digits and yields (0, 0, 0) for a string with no numbers at all. Harmless for a floor (0.0.0 never blocks) but wrong for a range, where the same leniency turned an unreadable spec into a *refusal*: a manifest whose only entry was garbage got compared against 0.0.0 and refused. Range specs are now shape-checked first, so garbage reads as "no evidence". parse_semver itself is unchanged, since the loader depends on its behaviour. Verified: 815 core unit tests pass, 18 of them new. Swept the real registry -- all 42 published manifests, at cores 1.0.0 / 2.0.0 / 3.1.0 / 3.2.0 / 4.0.0 -- and nothing is refused at any of them. The gate stays inert for shipped plugins, which is the property that makes it safe to land ahead of B5. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * feat(store): protect the one population the sunset would break The B6 sunset deletes each plugin's guarded-import fallback, so a plugin that floors at 3.2.0 must never reach a core that lacks the 3.2.0 modules. The gate could not stop that for the population most at risk. A device installed from the v3.1.0 release reports __version__ = "1.0.0". The gate treated anything below TRUSTWORTHY_FLOOR as "unknown, do not block" -- correct while every manifest floors at 2.0.0, because blocking would have emptied the plugin store for those users. But after the sunset it hands them a 3.2.0-floored plugin with no fallback, which fails to load with one log line. Nothing else in the system protects them: they cannot be told apart from a genuine 1.0.0 install. On an untrustworthy core the gate now refuses a floor ABOVE 2.0.0 and still allows anything at or below it. A floor above the ecosystem baseline says the plugin needs modules that arrived after 2.0.0, and a core reporting below that -- whether it is the v3.1.0 release or something genuinely ancient -- will not have them. Refusing leaves the user on the version they already run instead of one that cannot load. Measured against all 42 published manifests: today (every manifest floors at 2.0.0) core 1.0.0 / 2.0.0 / 3.1.0 / 3.2.0 / unparseable -> 0 of 42 refused after B6 (same manifests floored at 3.2.0) core 1.0.0 -> 38 refused, core 3.1.0 -> 38 refused, core 3.2.0 -> 0 So nobody loses the store today, and the sunset cannot reach a core that cannot run it. Two older tests asserted the previous "allow everything" behaviour; they now express the new rule with a 2.0.0 floor, which is what their no-lockout intent was actually about. The 38-of-42 in that measurement surfaced a separate B6 trap, recorded here because it will bite whoever raises the floors: four plugins (flights, leaderboard, music, stocks) declare the floor as a TOP-LEVEL `min_ledmatrix_version`, a third spelling, which declared_min_version checks before the versions[] array. For those, editing versions[0] is a silent no-op and the floor stays at 2.0.0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * fix(store): address review — malformed manifests, and suffixed versions Both CodeRabbit findings on #433 verified against the code and fixed. 1. Malformed manifest sections raised instead of degrading. `requires` as a list hit AttributeError ('list' object has no attribute 'get') and `versions` as a mapping hit KeyError: 0. Both reproduced. This got worse with the sunset rule in the previous commit: that branch resolves the floor for *every* manifest on an untrustworthy core, where the old code returned early. One hand-edited or third-party file with the wrong shape would have taken down the whole install path rather than just itself. Container types are now validated and an unrecognised shape reads as "no declared floor". 2. Prerelease and build metadata leaked into the version numbers. The digit scrape parsed "3.2.0+build42" as (3, 2, 42) and "3.2.0-rc1" as (3, 2, 1) -- a release candidate ranking above its own release. Both fed reject decisions, and the consequence was demonstrable: a plugin pinned to exactly "3.2.0" refused a core running 3.2.0+build42, which is that same version. The suggested remedy -- use the strict token parser -- would not have fixed it. _parse_strict validates the shape but delegates the numbers to parse_semver, so it returned the same (3, 2, 42). The bug is in the scrape, so suffixes are now dropped before it. Prereleases compare equal to their release rather than below it; full prerelease ordering is more than any caller needs and equal is far closer to right than what it did before. parse_semver is shared with PluginLoader, so its suite was re-run: unchanged, and it only ever gets more correct here. Verified: 839 core unit tests pass, 21 of them new -- six malformed shapes, five suffixed forms, and the two demonstrated regressions. The real-registry sweep is unchanged at 0 of 42 refused across cores 1.0.0, 3.1.0, 3.2.0, 3.2.0+build42 and an unparseable string. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-04 13:46:05 -04:00
ChuckandClaude Opus 5	970ca2d04f	feat(store): refuse to install a plugin that needs a newer core (re-target of #429) (#431) ... * feat(store): refuse to install a plugin that needs a newer core `ledmatrix_min_version` was decoration. The loader logged an advisory warning and continued; the store never compared the core version at all, so a routine "update" delivered a plugin that could not run. That is the gap phase B6 (the sports-unification sunset) cannot be done over: deleting a plugin's bundled fallback while nothing enforces the floor hands un-updated users a scoreboard that raises ModuleNotFoundError at load and is reported only as one line in the journal. The gate lives in install_plugin, after the manifest is on disk and before dependencies are installed. That is the earliest knowable point -- the registry carries no compatibility field, so the floor is not visible until the files are down -- and it is also the chokepoint: _reinstall_with_rollback calls install_plugin, so a refused *update* restores the version the user already had, for free. Floor resolution and the comparison move to src/plugin_system/compatibility.py, shared with the loader so the two cannot drift. Both read all four spellings published manifests use, including the deprecated `ledmatrix_min`. Refusal requires evidence. An undeclared floor, an unparseable version on either side, or a core below TRUSTWORTHY_FLOOR (2.0.0) all allow the install. That last one is deliberate and load-bearing: the v3.1.0 release reports __version__ = "1.0.0" while nearly every published manifest floors at 2.0.0, so a strict gate would lock those users out of the plugin store entirely -- much worse than the problem being solved. They stay unprotected until they update the core, which is also what fixes their version string. Verified: 782 core unit tests pass, including 25 new ones and the existing loader-warning suite unchanged (the refactor is behavior-preserving). The install tests drive the real install_plugin path with the download stubbed -- the allow and refuse cases differ only in the declared floor, so the refusal is demonstrably the gate and not an earlier bail-out. Follow-ups, deliberately not in this PR: surfacing the reason in the store UI rather than only the log, and publishing the floor in plugins.json so the store can refuse before downloading. Phase B4 in docs/SPORTS_UNIFICATION.md. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * fix(store): a failed install must not destroy the plugin it replaced Found while validating the compatibility gate. `_install_plugin_impl` deletes the existing plugin directory *before* downloading, so any failure after that point leaves the user with nothing. `_reinstall_with_rollback` protects the update path exactly this way; a direct `install_plugin` had no equivalent. The gate made this reachable in a new way: a plugin whose declared floor exceeds the running core is now refused *after* the old copy is already gone. Floors are hand-written and can be over-declared, so the refusal could remove a plugin that had been working fine on that core. install_plugin is now a thin wrapper that renames any existing install aside, delegates to _install_plugin_impl, and restores it on failure -- including when the implementation raises, which is re-raised after the restore. It is a pass-through when nothing is installed and when called from _reinstall_with_rollback, which has already moved the old copy aside; a test pins that so the two mechanisms cannot start nesting. The aside name embeds '.standalone-backup-' because plugin_manager._scan_directory_for_plugins keys on exactly that substring to skip backups. A different name would have made the backup discoverable as a duplicate plugin; a test pins that too. 789 core unit tests pass, including 7 new ones. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * fix(store): serialize concurrent installs, and make the lock reentrant Second bug found while validating the previous commit on hardware. install_plugin's new set-aside/restore had no lock. The web UI runs Flask threaded, so a double-clicked Install button gives two threads the same plugin_id; interleaved, one thread's restore deletes the other's freshly installed copy. _reinstall_with_rollback already guards exactly this with a per-plugin lock, and install_plugin needs the same one. Taking that lock naively deadlocks. _reinstall_with_rollback holds it across its call to install_plugin, and threading.Lock is not reentrant -- so the request thread hangs forever on the standard monorepo update path (update_plugin -> _reinstall_with_rollback -> install_plugin), which is to say on every plugin update. Verified by reverting to a plain Lock: the regression test times out after 10s instead of passing. The per-plugin locks are now RLocks, and install_plugin holds one for its whole set-aside/install/restore sequence. Verified on devpi (Pi, Python 3.13.5, real registry and network): - update_plugin on an up-to-date plugin: True in 5.4s - update_plugin forced through the full reinstall-with-rollback path: True in 13.1s, correct version restored, old copy replaced, no backup directories left behind - install -> reinstall-over-existing -> failed-reinstall-restores: all pass against real downloads - 22 plugins load, no tracebacks, web API and UI 200, steady-state journal 50 lines/min 791 core unit tests pass, including 2 new concurrency tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 * ci: run the new suites, and check tag/version agreement at release time These were split out of #428/#429 because the token pushing them lacked the `workflow` scope. Folding them in here rather than opening a stacked PR -- #429 was merged into its stacked base after that base had already been squash-merged, so its content never reached main, and one such near-miss is enough. All three enrolled suites exist on this branch: test_version_consistency.py came with #428 and is on main; the other two arrive with the commits above. Enrolling them in a separate PR would have either raced with this one on test.yml or briefly pointed CI at files main did not have. - test.yml: enroll test_version_consistency, test_plugin_compatibility_gate and test_install_preserves_existing in the core unit job. Until now these 32 tests existed but nothing ran them automatically. - release-version-check.yml: run scripts/check_release_version.py on pushed v* tags and published releases, plus workflow_dispatch so a tag can be checked *before* it is created. No dependencies -- it reads src/__init__.py and CHANGELOG.md only. Verified: both workflow files parse, and the release check still passes for v3.2.0 against this tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5 --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-08-03 16:52:42 -04:00
ChuckandClaude Fable 5	82a65ad2a2	Sports unification phase 0: safety net, cwd-independent fonts, element_style (#425) ... * fix(fonts): resolve asset paths against the install root, not the cwd FontManager built its catalog from cwd-relative paths ('assets/fonts'), so any process started outside the install root — the plugin safety harness on CI being the recurring case — found no fonts and silently degraded every plugin to PIL's default face. Several plugins grew per-plugin workarounds for exactly this (countdown, text-display, tide-display in the plugins monorepo). Catalog population now falls back to the install root derived from this module's location when the cwd-relative path is missing; behavior when running from the install root is unchanged. Verified: resolve_font returns the real FreeType face from a foreign cwd, and the full unit suites (266 tests) pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4 * docs: seed CHANGELOG.md with the module-availability release discipline The plugins monorepo's sunset rule ('delete a bundled fallback copy only when the manifest floors on the first core release shipping the module') needs core module additions recorded against version numbers. Seeds the changelog at 3.1.0 and documents the discipline. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4 * ci: enroll the core unit suites in a dedicated job The existing workflow ran only the three plugin-harness suites; the skin-system, font-manager, data-source, extractor, scroll-helper, adaptive-layout, and loader-compat suites (266 tests) existed but never ran in CI, so a refactor of src/base_classes or src/common could regress them silently. Also enrolls the new sports characterization and element-style suites landing in this branch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4 * feat: ship src/element_style — the per-element style resolver plugins already expect Three plugins (of-the-day, ledmatrix-music, football-scoreboard) import src.element_style behind guarded try/except with classic fallbacks, but the module never existed in core, so the richer per-element styling UI those code paths implement has been dormant. This lands it: - ElementStyleResolver.style() resolves per-element font/size/color with the key semantic the consumers encode: a config value counts as user-forced only when it differs from the schema default (the web UI bakes defaults into config.json on save), and untouched configs resolve to exactly the caller's classic values — byte-identical rendering, proven by of-the-day's committed goldens passing unchanged. - defaults_from_schema_file parses both declaration forms (the compact x-style-elements map and hand-written customization blocks). - expand_style_elements() expands x-style-elements into full config blocks; schema_manager.load_schema() applies it (guarded, no-op for schemas without the declaration) so the config form and defaults merging see the expanded UI. - Fonts resolve cwd-independently with (path, size) caching; .bdf loads via freetype like FontManager; nothing in the module raises out of style(). Verified: 31 new unit tests; of-the-day's previously-skipped 9-test spec suite now runs and passes; football's resolver tests pass (27); music's 38 plugin tests pass; schema-manager suites pass (43). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4 * test: characterization suite for src/base_classes/sports.py ahead of unification Pins current behavior before the planned merge of the nine drifted plugin copies back into this ancestor: the _extract_game_details_common key contract per sport (reusing GUARANTEED_KEYS from the skin tests), update() flows for upcoming/recent/live against cache-seeded fixtures under frozen time, rendering smoke per mode class, and guard rails on the skin-system seam. Five surprising behaviors are pinned AS-IS and flagged in comments so the merge changes them knowingly or not at all: is_upcoming also matching status.type.name; hockey dropping events whose competitors lack 'statistics'; baseball reading the event-level status for innings; no past-date filter in upcoming; and favorites-only mode with an empty favorites list showing nothing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4 * ci: restrict the test workflow's GITHUB_TOKEN to contents:read CodeQL flagged the new unit-tests job for running with the default unrestricted token; the pre-existing job had the same exposure. Both jobs only check out the repo and run pytest, so a workflow-level contents:read is sufficient. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FgbA8SMutQQpXkMG8LMmC4 * Address CodeRabbit review: font-name traversal + offline test guard Two Minor findings from CodeRabbit's first review of this PR. - resolve_font_path: reject relative font names carrying path components. font_name comes from plugin config, which the web UI writes; a value like "../../config/config.json" escaped assets/fonts/ after os.path.join and let a config probe arbitrary paths for existence (disclosure unlikely, since Pillow/freetype reject non-font files, but the probe is real). Relative names must now be bare filenames (os.path.basename(name) == name); absolute paths keep their existing isfile() gate. Test confirms the traversal resolved the real config.json before the guard. - build_manager fixture: patch requests.Session.get BEFORE constructing the manager. Construction creates both SportsCore.session and the ESPNDataSource.session; the old code only replaced manager.session after the fact, leaving data_source.session real and able to reach the network on an accidental fetch. Patching the class makes every session built in the fixture offline. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-08-02 12:01:56 -04:00
ChuckandClaude Sonnet 5	83f20b64fe	Give plugins access to device-wide config, and a global scroll frame rate (#424) ... * Give plugins access to device-wide config, and a global scroll frame rate The sports scoreboards read `getattr(self, 'global_config', {})` to find a shared scroll frame rate, but nothing ever set that attribute: the loader constructs plugins with only plugin_id/config/display_manager/cache_manager/ plugin_manager (plugin_loader.py:671), `global_config` appears nowhere in src/, no plugin manager assigns it, and BasePlugin has no __getattr__ to synthesize it. The lookup always returned {}, so the ten scroll_display.py copies that thread target_fps through to ScrollHelper could never fire on any core. There was also no global target_fps to find -- the only one in the template is display.vegas_scroll.target_fps, which is Vegas-scoped. Adds the missing half: - `BasePlugin.global_config` resolves the full config via plugin_manager.config_manager, then cache_manager.config_manager, then {}. Same order the sports timezone helpers already use. Exceptions are swallowed to debug so an unreadable config can never stop a plugin loading, and a non-dict result is rejected rather than handed to callers that will .get() it and feed the result to numeric code. - A top-level `target_fps` (default 100), exposed on the General tab and validated 30-200 on save to match ScrollHelper.set_target_fps -- which clamps silently, so a rejected save reports a value that would otherwise appear to save and then behave differently. The property has a setter deliberately. news, stock-news, ledmatrix-stocks, ledmatrix-elections, ledmatrix-leaderboard and nfl-draft all assign `self.global_config = config.get('global', {})`; without a setter that raises "property has no setter" and those six plugins stop loading. Reproduced, then pinned with a test. target_fps is also kept out of the `is_general_update` key list: that branch treats a missing web_display_autostart as an unchecked box, so counting a target_fps-only POST as a General save would silently switch autostart off. Verified end to end: config.json -> BasePlugin.global_config -> scroll_display's existing block -> ScrollHelper.target_fps 120 -> 100, with no plugin-side change needed. Suite 1441 passed; the 4 failures (test_display_dirty_tracking, test_web_api::test_get_system_status, two in test_state_reconciliation) are pre-existing and reproduce identically on a clean tree. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Address review findings on target_fps validation and config resolution - Reject floats and bools before int() in the target_fps save path. A JSON body can carry them, where int(90.5) silently stored 90 and true stored 1. Form posts send strings, so '90.5' already failed in int(). - Assert the template's target_fps is 100, not merely an int, so the documented default is actually pinned. - Empty-config precedence: keeping the `and config` check deliberately, now spelled out in the comment and covered by a test. Both managers default to the same config/config.json, so falling through cannot pick up a different file's settings; treating {} as an answer would instead return {} when the first manager simply hasn't loaded yet, silently disabling every setting read through the property -- the failure this property exists to fix. Suite 1446 passed. The float-rejection test was checked to fail without the guard. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>	2026-08-02 10:40:12 -04:00
ChuckandClaude Sonnet 5	5b45f35888	Vegas mode: reclaim dead space and pace the rotation (#423) ... * Vegas mode: reclaim dead space and pace the rotation On a wide panel Vegas mode spent much of its time showing black. At 50px/s on a 512px display, one display width of blank is 10.2 seconds, which makes several long-standing behaviours expensive: - ScrollHelper prepended a full display width of black as an "initial gap", charged once per cycle — 10.2s of black at the start of every rotation. - Plugins without get_vegas_content() are captured off a full-display canvas, so their blank margins entered the ticker too. Measured: of-the-day drew 35px of "No Data" on a 512px canvas (92% blank), youtube-stats 142px of content with 185px of black either side. Only the scroll_helper path had any trimming. - Cycle transitions deliberately pushed a blank frame and then recomposed synchronously: 84ms at best, 4.8s at worst, every millisecond of it black. - buffer_ahead doubled as the cycle size, so a 21-plugin install showed 3 plugins per cycle and took ~7 cycles to come around. - separator_width was applied between every image rather than at plugin boundaries, so a per-row ticker like the F1 scoreboard (116 images, which it renders 4px apart internally) got a 32px chasm between each row — and the width budget didn't count those gaps, so the plugin quietly occupied far more of the panel than intended. Changes: - src/vegas_mode/geometry.py: numpy column-ink primitives shared by the trimmer and the audit tool, so the number reported is the number acted on. A Python per-column loop over a 17,000px strip is far too slow for the render path. - PluginAdapter trims every content path, not just scroll_helper. Only outer edges are cropped: interior blank columns are the plugin's own layout (logo left, score right) and closing them would corrupt the design. A plugin on a non-black background is inherently unaffected. - ScrollHelper.create_scrolling_image takes an explicit lead_gap, still defaulting to display_width so the many standalone-ticker callers are unchanged. Vegas passes lead_in_width (default 0). - Cycle end holds the last rendered frame instead of blanking, turning the recompose into a brief freeze rather than the panel switching off. - plugins_per_cycle (default 6) is split from buffer_ahead, which goes back to being only a prefetch low-water mark. - max_plugin_width_ratio (default 3x display width) caps one plugin's share of a cycle. Overflow is deferred, not discarded: a rotation offset advances each fetch so later rows appear on subsequent cycles. Single oversized images are cropped at a blank column so the cut misses glyphs. - Composition groups images by plugin: rows are joined by intra_plugin_gap (default 8) and separator_width applies only between plugins. The width budget now counts those gaps. - Plugin data updates no longer run on the Vegas render path. All new settings are user-configurable in Display -> Vegas Scroll, including min/max cycle duration and dynamic duration, which previously existed in code but were reachable only by hand-editing config.json. Measured with scripts/dev/vegas_audit.py on a 512x64 panel: mean ink coverage 42.7% -> 69.4% fully blank 5.9% -> 0% reads as empty 13.6% -> 0% worst blank stretch 4.8s -> 0s full rotation 414s -> 123s plugins per cycle 3 -> 6 Note the metric choice: a "fully blank" scan (>=95% black viewport) reported only 0.4% and badly understated the problem, because two full-width segments with mid-canvas content never fully blank the viewport — they hold it at ~28%. window_coverage_stats grades every viewport position by how much ink it carries, which is what tracks perceived dead time. Known remaining: cycle transitions still freeze ~3.5s while the next cycle is fetched. Fixing that needs background prefetch, which is deferred because the fallback-capture path mutates the shared display_manager.image and racing it against the render loop risks torn frames. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Drop unused Optional import from the vegas audit script Flagged by Codacy (F401). Any, Dict and List are all still used. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Align Vegas API bounds with validate(), fix audit config plumbing Both from review feedback on #423. The web API's accepted ranges disagreed with VegasModeConfig.validate(), which is what actually gates Vegas starting: scroll_speed 1-100 -> 1-200 (a slider value of 150 returned 400) separator_width 0-500 -> 0-128 target_fps 1-200 -> 30-200 buffer_ahead 1-20 -> 1-5 The three loose ones were the dangerous direction: the value saved with a 200, then VegasModeCoordinator.start() failed validation with only a log line, so the ticker silently never ran. The UI already matched validate() in all four cases, so the API was the odd one out. test_vegas_api_bounds_match_validate parses the numeric_fields map out of api_v3 and asserts every bound against validate(), plus that validate() accepts both endpoints and rejects just outside them, so these cannot drift apart again. That test immediately caught a missing upper bound on min_plugin_width, now added — unbounded it would drop every segment and leave a blank ticker. Separately, vegas_audit.py constructed PluginAdapter without the config, so it fell back to VegasModeConfig() defaults and would report trimming and width-budget behaviour that differed from the user's config.json. It now passes the loaded config exactly as the coordinator does. This is the same class of drift the explicit lead_gap and grouping arguments already guard against. Output is unchanged on a rig whose config matches the defaults. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Vegas mode: render plugins narrower, space rows by measured separation Trimming reclaims blank margins but cannot compact a layout that genuinely spans the display — a five-column forecast, a progress bar drawn at 100% width, a stat block with the panel's whole width between its elements. Those need the plugin to make different layout decisions, which means telling it the screen is narrower while it renders. DisplayManager.render_size() presents a smaller logical canvas for the duration of a Vegas content fetch, reusing the same _LogicalMatrix indirection double-sided mode already relies on so plugins see a consistent size from every accessor. Plugins that size themselves from matrix.width need no changes at all; one that wants to be explicit can read the new BasePlugin.get_vegas_render_width(). Width is a percentage so a single setting travels across panel sizes: vegas_scroll.render_width_pct globally, or vegas_width_pct in an individual plugin's config. Measured on a 512x64 panel with real data: ledmatrix-weather 1536px -> 576px (forecast becomes narrow cards) youtube-stats 353px -> 199px (2% blank left, so genuinely compact) geochron 453px -> 153px (ink density rises to 100%) ledmatrix-flights 950px -> 740px The youtube-stats figure is the clearest evidence the layout itself changed rather than being cropped: at full width the content had to be trimmed from 512px to 353px, whereas at 40% it arrives with almost no blank to reclaim. Row spacing is now measured rather than added. A flat gap gets it wrong in both directions at once — content drawn flush to its own edges ends up nearly touching (reported for recent sports scores, which sat 8px apart), while content already carrying wide margins gets pushed even further out. separation_gap() measures the blank each pair already has and adds only the shortfall, up to min_content_separation (default 24). intra_plugin_gap stays as a floor applied regardless. Two tests shipped in the previous commit encoded the old flat-gap arithmetic and are updated to the measured semantics, including one renamed to reflect that zero intra_plugin_gap alone no longer butts rows together. Also fixes a real bug found while testing: the harness display manager had no render_size(), and because the adapter catches broadly that surfaced as "no content" rather than an error, silently dropping five plugins. Added the context to VisualTestDisplayManager for parity, and _render_at() now degrades to a no-op on any display manager lacking it, so a third-party or older harness loses the narrowing rather than the content. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Vegas mode: end cycles before the wrap, keep the width budget honest Three fixes, the first a regression from lead_in_width defaulting to 0. get_visible_portion wraps: once scroll_position + display_width passes the end of the strip it fills the right of the frame from the *head* of the same strip. So the final display_width of travel showed the cycle's first plugin re-entering on the right while its last plugin exited on the left, and the recompose that followed replaced both at once. On a 512px panel at 50px/s that was 10.2s of two plugins on screen at once, ending in a hard cut — reported as the ticker "switching mid-scroll" from F1 to news. That used to be invisible because the strip began with a full display_width of blank, so the wrapped-in region was black. Removing that blank (it was 10s of dead panel per cycle) exposed the wrap. Cycles now end one display width earlier, before any wrapped content appears, clamped for strips no wider than the display so they don't complete instantly and spin the recompose loop. Verified on hardware: a 3936px strip now completes at 68.5s, exactly (3936 - 512) / 50. Second, auto_trim=False also skipped the width budget, which is an unrelated concern — turning off margin cropping should not let one plugin hold the panel for minutes. Seen in the field: the F1 scoreboard contributed 116 images and 14,848px untouched, giving a 33,821px cycle (11 minutes of content). The budget now applies regardless of trimming; with it restored that cycle is 6,362px. Third, the budget accounted for row gaps using the flat intra_plugin_gap while the compositor had moved to measured separation, so it under-counted by up to (min_content_separation - intra_plugin_gap) per row and a many-row plugin overran its cap. Both now use the same separation_gap() rule, and a test asserts the composed block fits the budget end to end rather than trusting the two paths to agree. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Fix IndexError in find_blank_cut when the cut lands on the image edge A cut position after the last column is legitimate — _crop_to_budget asks for min(start + budget, img.width), which equals the width whenever the remaining strip is shorter than the budget. find_blank_cut clamped target to width but then walked leftwards starting at target itself, so ink[width] raised IndexError. Caught on hardware: it killed the ledmatrix-stocks fetch, and because _fetch_plugin_content catches broadly that surfaced as the plugin silently contributing nothing for the cycle. Only reachable on the second or later pass of the rotating window over a single oversized image, which is why the existing tests missed it — they all exercised the first pass, where start is 0 and start + budget is comfortably inside the image. Added TestRotationAcrossMultipleCycles, which walks the window round several times and asserts content is never lost, plus direct coverage of find_blank_cut at and beyond the image edge. Both bounds now stop at width - 1 so neither direction can index past the end. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Only cut oversized segments at real gaps between items The width-budget crop snapped to the nearest blank column, and in rendered text the gap between two characters is a single column. So a cut routinely landed inside a word: the cycle showed "Wednesda" and the orphaned "y" turned up as a lone floating letter in the next cycle, positioned after whatever plugin happened to precede it. Measured on the clock-simple segment to confirm: its blank runs are [1, 1, 1, 1, 1, 8, 8] — five single-column letter gaps, every one of which find_blank_cut would happily have chosen. Cuts now only land in a run of at least min_cut_gap blank columns (default 6), which excludes letter spacing while still finding the gaps plugins put between items (the stocks ticker uses 32px, baseball 48px). Where no boundary falls inside the budget the cut waits for the next one and overruns, because splitting an item is worse than a slightly long segment. Continuous content is treated differently on purpose: an image with no internal gaps is a map or a chart, where any column is as good as another, so it is still cut to the budget exactly. The gap rule protects discrete items; letting a solid image escape the cap in its name would be wrong. blank_runs() is vectorised — 48ms for a 17,000px strip, against seconds for a per-column Python loop. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Hold capture_mode for every plugin render, not just narrowed ones The native content path only entered capture_mode when it was also narrowing the canvas, so at full width — which is every plugin without a vegas_width_pct override, i.e. most of them — a plugin calling update_display() while building its Vegas content wrote straight to the hardware. That is a visible flash mid-scroll, and it lines up with the flash reported at cycle transitions, when several plugins are fetched back to back. Suppression is now unconditional; the narrowing context stays separate because it is already a no-op at full width. Both contexts are reached through helpers that degrade to nullcontext when the display manager lacks them. That matters more than it looks: the adapter's handlers are deliberately broad, so an AttributeError from a missing context does not surface as an error — it surfaces as the plugin contributing nothing. Making the call unconditional without this turned 44 tests red for exactly that reason, all of them reporting lost content rather than the real cause. The test double now provides capture_mode and render_size too, so tests exercise the real contexts instead of silently taking the degraded path. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Vegas mode: one continuous strip instead of swapping cycles A cycle used to be a discrete strip that got replaced: motion stopped, every pixel was substituted at once, and the next group started with the viewport already full. That is the freeze, the flash and the jump. The strip is now extended rather than replaced. ScrollHelper gains append_content(), which adds items on the right without touching scroll_position or total_distance_scrolled, so motion continues and the next group simply arrives from the right. Because completion is measured against total_scroll_width, extending also defers completion — there is no longer a cycle boundary to see. drop_scrolled_prefix() reclaims what has gone past, keeping the strip bounded however long Vegas runs (observed 5,000-11,000px against an unbounded strip otherwise). It shifts total_distance_scrolled and total_scroll_width together so the completion arithmetic is unchanged, and refuses to run while the viewport is wrapping: wrapping reads the head of the strip into the right of the frame, so trimming the head there would visibly change the picture. A test caught that. Groups are prepared off the render thread. The constraint is that the canvas and the matrix proxy are process-wide mutable state, so narrowing or capturing through them from another thread would corrupt the frame the render loop is pushing. get_content() therefore takes offscreen_only: the background thread uses only paths that avoid the canvas, and anything needing it is marked and picked up on the render thread. That puts the expensive work (native renders of leaderboard and baseball cards, seconds each) in the background and leaves the cheap work (display capture, 40-600ms) in the foreground. DisplayManager's capture flag is now thread-local. As a shared flag, a background capture would have suppressed the render loop's own frame pushes for its duration, freezing the panel precisely when the point was to avoid a freeze. Canvas-bound plugins are drained one at a time rather than as a batch: six at once held the render thread for 1.75s. Drains are also spaced by two seconds while the lookahead is healthy, since taking them back to back turns one long stall into a run of short ones. When the strip is genuinely running short the throttle is ignored, because content matters more than smoothness there. Measured on hardware: zero cycle-complete swaps, drains landing 2-4s apart, lookahead holding at 1,200-3,500px, no errors. Set continuous_scroll false to restore the swap behaviour; the old path is intact. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Pace the Vegas frame loop adaptively: 31.5 -> 78.7 fps The loop slept a fixed frame_interval on top of however long the frame took, so at a measured 31.6ms per frame a flat 8ms of that was pure idle — a quarter of the budget spent not rendering. It now sleeps only the remainder of the budget. Measured on hardware: 31.5 fps to 78.7 fps sustained, with CPU going *down* from 150% to 127%. Scroll speed is unchanged at 49.9px/s against a configured 50, because motion is derived from elapsed time rather than frame count — this buys smoothness, not speed. Worth recording what the bottleneck was not: the per-frame render path measures 0.34ms in total (0.18ms for the numpy slice, 0.17ms for the dirty-tracking digest), which is a theoretical 2900 fps. Optimising any of that would have been wasted effort. The frame was idle, not busy. Also nices the prefetch thread. Its work is PIL and numpy that releases the GIL, so the scheduler can act on the priority, and without it the prefetch competes for the same cores as the render loop. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Sub-pixel scrolling: motion at the frame rate, not the pixel rate With integer positioning the number of distinct frames per second equals the scroll speed in px/s, however fast the loop renders. Measured at 50px/s and 78.7fps, 36% of frames were byte-identical: the extra frames cost work and bought no motion, and what was left was 50 discrete 1px steps a second. Two things were wrong with the pre-existing sub-pixel support. get_visible_portion never consulted sub_pixel_scrolling — it always took the integer path, so the flag and _get_visible_portion_subpixel were dead code. And that implementation needed scipy.ndimage.shift, which is not installed on the target devices (HAS_SCIPY is False there), so it would not have interpolated even if reached. Verified both: positions 1000.0 and 1000.5 produced identical frames either way. Blending is now wired up and implemented with numpy. Two details make it affordable: slice cached_array directly instead of building two PIL images only to convert them straight back (the naive version measured 15x the integer path), and use fixed-point uint16 multiply-add rather than float32, which suits the Pi's cores and gives finer weighting than the panel can resolve. Result 0.939ms against 0.237ms — 0.70ms added per frame, a 1065fps ceiling. Measured on hardware: 81.2 fps with blending on, against 78.7 with it off, so no cost within noise — and every frame is now a distinct position rather than one in three being a repeat. The trade is a slight horizontal softening of text, since each frame blends two positions. Set smooth_scroll false for maximum crispness. Also benchmarked and cleared as non-issues: extending the strip costs 9.4ms on an 11,000px strip and trimming 2.5ms, both under one frame at this rate. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Add overflow handling: keep ordered content whole instead of rotating a window The width budget split any oversized plugin by advancing a window each cycle. That is right for interchangeable items — news headlines, odds, stock prices — but wrong for ordered content: a league table showed ranks 1-6, then resumed at 7 two rotations later, which reads as out of order and out of context. Nobody needs rank 23 in a ticker; they need the top of the table, every time. overflow_mode chooses between them: rotate — advance a window each cycle so everything is seen eventually (unchanged default) truncate — always show the start and drop the rest, keeping ordered content coherent. Records no window state, so every pass starts at the top. Per-plugin vegas_overflow overrides the global setting, since one install has both kinds of plugin. Also adds per-plugin vegas_max_width_screens, so content that must stay whole can be given more room — or uncapped with 0 — without lifting the cap on every ticker. Applied on the test rig: f1-scoreboard and ledmatrix-leaderboard set to truncate, and baseball given 4.5 screens because it was showing 8 of 9 games when the whole slate needed only a little more room. Verified: F1 now reports "the first 10 of 116 ... the rest are not shown", baseball has dropped out of the budget log entirely, and stocks, odds-ticker and stock-news still rotate. Also corrects the crop log, which claimed "window advances next cycle" unconditionally and so misreported truncated crops. A test now pins the behaviour behind the message: truncate must leave no offset recorded. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Stop Vegas mode showing last night's games as if they were live A game that was live in the evening was still being drawn as live the next morning. Two faults combined to freeze plugin visuals indefinitely. PR #291 added a call to plugin_adapter.invalidate_plugin_scroll_cache() so a plugin's own cached scroll image would be rebuilt from fresh data. That method was never implemented. hot_swap_content() wraps the call in a broad except, so every hot swap has raised AttributeError and been swallowed silently ever since — which is why the visuals it was meant to keep fresh never were. Continuous scrolling then removed the only path that reached it at all: should_recompose() and hot_swap_content() are called from the non-continuous branch of run_frame(), and continuous_scroll defaults to True. So on a default install the pending-update flags were set by the update tick, never consumed, and grew without bound. Together these froze content completely, because refetching is not enough on its own: the sports plugins' get_vegas_content() regenerates only "if the cache is empty", so take_next_group() kept receiving the same picture however often it asked. Fixed by: - Implementing invalidate_plugin_scroll_cache(). It covers both layouts — a helper directly on the plugin (stocks, news, odds-ticker) and one owned by a scroll-display manager (the sports scoreboards, which is the shape that produced this bug) — and clears cached_image and cached_array together, since the array is the image's numpy mirror. - Adding StreamManager.invalidate_pending_updates() and calling it from the continuous branch. It only drops the caches; the plugin recomposes when it next comes round in the rotation. process_updates() is wrong here: it refetches synchronously and merges into the active buffer that continuous mode bypasses, and hot_swap_content() rebuilds and repositions the whole strip, which is the freeze-and-jump this mode exists to avoid. Tests assert the fix rather than the implementation: 14 of the 17 new tests fail without it. Includes the wiring itself, since the regression was a call that was simply absent, and a check that the scroll position is untouched so this cannot regress into the swap's visible jump. All Vegas suites pass (355 tests). test_display_controller_vegas_tick.py still cannot be collected off-device for want of rgbmatrix, identically with and without this change. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * Fix two CodeRabbit-flagged test assertions in vegas density tests test_prepared_group_is_used_without_refetching had a tautological final assertion; now checks stream.calls directly. test_no_partial_letter_at_either_edge required both crop edges to be blank, but the left edge here is always the crop's start position with no lead-in gap in word_strip, so it legitimately carries ink — only the right edge is an actual cut and needs the check. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-31 09:40:38 -04:00
ChuckandClaude Fable 5	cdf03fb107	Add skin system: user-installable visual overlays for sports scoreboards (#419) ... * Add skin system: user-installable visual overlays for sports scoreboards Skins restyle a scoreboard's live/recent/upcoming rendering while the host plugin keeps doing data fetching, scheduling, caching, live priority, and vegas mode — the anti-fork alternative for users who only want a different layout. - src/skin_system/: ScoreboardSkin API, SkinContext (canvas + adaptive layout + logo/font helpers), discovery/loading runtime with API major version gating and per-skin module namespacing - src/base_classes/sports.py: _render_game() seam at the three _draw_scorebug_layout call sites; skin-first with built-in fallback, 3-strikes session disable, slow-render warning; per-mode skin config - scripts/validate_skin.py: headless multi-mode/multi-size validator with bundled per-sport fixtures (no hardware or network needed) - skins/example-classic-baseball/: working reference skin - Web UI: served-schema Visual Skin dropdown (validation never enum-restricted, so uninstalled skins can't invalidate configs) and GET /api/v3/skins - Store: registry entries with type "skin" install to skins/ - docs/SKIN_SYSTEM.md (architecture), docs/CREATING_SKINS.md (author guide incl. Claude Code prompt), view-model contract locked by tests Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LrCusPasy1qeUN5anK3aA1 * Address review feedback on skin system - skin_runtime: cache the entry module so the 2nd/3rd load of the same skin (live/recent/upcoming hosts) doesn't re-execute it with unbound sibling aliases; rebind cached sibling modules to their bare names around entry import and restore prior bindings after; include per- manifest mtimes in the discovery cache fingerprint so in-place skin updates are picked up - sports.py: count render_skin_card exceptions toward the 3-strike session disable - store_manager: validate skin ids (pattern + resolved-path containment in skins/), reject registry/manifest id mismatches, and stage+validate downloads in a temp sibling before replacing an existing skin - schema_manager: leave the schema untouched when the configured skin value is a per-mode mapping (a string dropdown could overwrite it) - validate_skin.py: reject non-positive sizes and non-object --options at parse time; support --output-dir outside the repo; type annotations - example skin: validate accent_color once at load with logged fallback - fixtures: pregame 0-0 scores in football/hockey upcoming fixtures - api /skins: rely on the self-invalidating discovery cache instead of force_refresh - docs: valid JSON manifest example, load_logo caching semantics spelled out, language ids on fenced blocks - tests: view-model contract test now exercises the real extractor; regression test for repeated same-skin loads with sibling modules Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LrCusPasy1qeUN5anK3aA1 --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-18 11:07:08 -04:00
ChuckandClaude	6a9d8014e5	Tag plugin logs structurally and surface the active plugin in System Logs (#418) ... - get_logger() now returns a PluginLoggerAdapter when given a plugin_id, so every plugin log call is stamped with plugin_id automatically instead of only calls that explicitly passed extra={'plugin_id': ...}. This makes the "[Plugin: x]" prefix reliable in the journalctl-backed log stream. - display_controller publishes the currently active mode/plugin to the shared cache whenever it changes, exposed via a new GET /api/v3/display/current-status endpoint. - System Logs page: adds a "Now showing" banner backed by that endpoint, a plugin filter dropdown (populated from parsed log lines), a plugin badge per log entry, and fixes log parsing to handle the short-iso timestamp format journalctl actually returns (the old regex only matched syslog timestamps, so level/plugin extraction silently never ran). Co-authored-by: Claude <noreply@anthropic.com>	2026-07-18 10:59:36 -04:00
ChuckandClaude Sonnet 5	6499794c12	fix(testing): add MockCacheManager.get_cached_data_with_strategy/save_cache (#409) ... * fix(testing): add MockCacheManager.get_cached_data_with_strategy/save_cache ledmatrix-leaderboard's data_fetcher.py calls these two real-CacheManager methods (src/cache_manager.py:313,817), but MockCacheManager had neither -- update() always hit an AttributeError, caught by a broad except and logged, so the harness rendered an empty-but-green leaderboard on every test run without ever exercising real standings data. Both mocks delegate to the existing get()/set() -- a mock doesn't need the real strategy's per-data-type max_age/market-hours timing, plugins under test just need the methods to exist and round-trip whatever was cached. * fix(testing): clear get_cached_data_with_strategy_calls in MockCacheManager.reset() reset() cleared get_calls/set_calls/delete_calls but not the newer get_cached_data_with_strategy_calls tracker, so a reused mock (e.g. across test cases sharing a fixture) retained stale strategy-call records after reset(). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>	2026-07-14 08:33:08 -04:00
Chuck	3d347a368a	fix(testing): stop plugin enabled:false schema defaults from silently disabling harness tests (#408) ... check_plugin.py, render_plugin.py, and the pytest plugin matrix each built config as {"enabled": True} then merged in config_schema.json's defaults on top, letting a plugin's own enabled:false default (a reasonable choice for a seasonal/opt-in plugin -- 15 of 23 real plugins ship one) silently win. Every harness/CI render of those plugins was testing "disabled, do nothing" rather than real behavior. Extract build_full_config() into testing/loading.py (already the shared home for plugin-discovery/config-default logic) and use it from all three call sites: schema defaults, then a forced enabled=True, then harness.json's config, then the caller's explicit config -- so a test can still deliberately disable a plugin on purpose, it just can't happen by accident via the plugin's own shipped schema default anymore.	2026-07-14 08:21:35 -04:00
3 people	0aca40cf3a	perf(plugins): run scheduled updates off the render thread (#407) ... * perf(plugins): run scheduled updates off the render thread plugin update() executed inline in the render loop — execute_update's internal thread.join(timeout=30) blocked it, so one slow plugin HTTP fetch froze scrolling for the whole fetch (up to 30s; DNS-retry storms made this a regular occurrence on flaky networks). Scheduling stays on the render thread and keeps every existing gate (enabled, circuit breaker, can_execute, interval); due updates are now enqueued to a single background worker (serialized — same one-at-a-time execution as before, no thundering herd). RUNNING is set at enqueue so can_execute blocks re-entry alongside the pending-set dedup. Per-plugin locks make the old implicit update/display no-overlap guarantee explicit: the worker holds the plugin's lock through its update; the display side try-locks and, when the plugin is mid-update, holds the last frame for that iteration — reported as success so a mid-update skip never advances the rotation. Unlike before, the guarantee now also holds across the post-timeout window (previously the lingering update thread overlapped display()). Deadlock-free by construction: the worker takes one lock; display never blocks. Timeout semantics unchanged (lingering daemon thread documented). Kill switch: plugin_system.synchronous_updates: true restores the inline path. 8 new concurrency tests (non-blocking scheduler, overlap assertion under a hammering display loop, lock release on failure/timeout paths, dedup, kill switch); 4-min devpi soak clean (updates completing, rotation advancing, no stuck RUNNING states). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam * fix(plugins): fix skipped-frame health/force_change tracking, config validation, and lock-lifetime gaps in async updates Addresses PR #407 review findings: - display_controller: only clear force_change / record health success when display() actually ran this frame, not when the frame was skipped because the plugin's lock was busy (a skip must preserve a pending mode-switch force_clear). - plugin_manager: replace bool() coercion of synchronous_updates with explicit isinstance validation of plugin_system/synchronous_updates, failing safe to synchronous mode (with a logged reason) on malformed config instead of silently defaulting to async. - plugin_manager: _update_worker_loop now acquires the plugin lock before looking up its instance and re-checks under the lock, so an unloaded plugin's lifecycle state is never resurrected to ENABLED. - plugin_manager + display_controller: move lock ownership (and, for updates, RUNNING/pending lifecycle bookkeeping) into the actual update()/ display() call itself rather than the timeout-wrapped caller, so the lock stays held for the real operation's duration even after PluginExecutor's own join(timeout) elapses and a lingering daemon thread keeps running in the background. - DisplayController.cleanup() now stops the update worker before tearing down display/cache resources; stop_update_worker() logs when the join times out instead of failing silently. - test_async_plugin_updates: rewrite test_unloaded_while_queued_is_harmless to exercise the public unload_plugin() lifecycle (via a deterministic blocker) instead of deleting pm.plugins directly, and add a regression test proving the plugin lock stays held through PluginExecutor's own timeout while the real update() call is still running. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-07-14 08:08:51 -04:00
3 people	c1fa5094be	fix(store): plugin updates keep the old install until the new one succeeds (#405) ... * fix(store): plugin updates keep the old install until the new one succeeds Both reinstall paths in update_plugin — the monorepo-migration remote switch AND the routine archive update every store user hits — deleted the installed plugin directory BEFORE downloading its replacement. A mid-update failure (bad network, registry error) permanently destroyed the plugin. Seen in the field: a Pi with broken DNS lost 12 plugins in one update pass during the monorepo migration. New _reinstall_with_rollback: rename the old install aside (using the '.standalone-backup-' name pattern plugin discovery already excludes), run install_plugin, remove the aside on success — restore it on ANY failure, clearing partial-download debris first. A stale aside from a previous crash is cleared before starting. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam * fix(store): serialize concurrent updates per plugin, check cleanup results CodeRabbit review on #405 flagged two things in _reinstall_with_rollback, both verified against current code: - Real race: the web UI runs Flask with threaded=True and there's a single update route, so two overlapping requests for the same plugin_id (double-click, two tabs) can interleave. The loser could rename the winner's in-progress install aside mid-download, deleting its own rollback safety net — worse than the bug this function exists to fix. Added a lazy per-plugin_id lock dict (mirrors the plugin_manager per-plugin lock pattern) held for the whole function. - _safe_remove_directory's return value was ignored at both call sites. Stale-aside cleanup failure now aborts cleanly instead of falling through to a rename that would fail anyway with a less useful error; post-success backup-removal failure now logs instead of failing silently (still returns True — the update itself succeeded, and the next update self-heals the leftover aside). Left the third nitpick (test_stale_aside_from_previous_crash_is_cleared) addressed by asserting the stale dir is actually gone and that install_plugin was reached, rather than just the end-to-end result. Added a concurrency regression test asserting install_plugin never runs for the same plugin_id while another call is in flight. --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-07-13 09:32:37 -04:00
ChuckandClaude	1c7a0cef66	fix(vegas): lock plugin_last_update snapshot/diff against concurrent mutation (#398) ... * fix(vegas): restore live plugin-update refresh dropped by sync refactor Investigating a user report that Vegas scroll mode doesn't update scores or game status. Root cause: PR #299 (Mar 28) added a mechanism so a live score change reached the ticker within a few seconds instead of waiting for a full scroll cycle -- _tick_plugin_updates_for_vegas() diffed plugin_last_update timestamps to detect which plugins got fresh data and called coordinator.mark_plugin_updated() for each, and should_recompose() checked has_pending_updates_for_visible_segments() to trigger an immediate hot-swap. PR #330 (May 14, multi-display wireless sync) refactored both call sites while adding sync support and silently deleted this entire mechanism -- not just gated it behind the new sync-mode deferral it legitimately needed, but removed it outright. The result: VegasModeCoordinator. mark_plugin_updated() and StreamManager.has_pending_updates_for_visible_ segments() have been fully implemented but never called from anywhere since. Vegas mode's only remaining freshness sources are a 5s content cache TTL (fine) and full recompose at cycle boundaries, which depending on min/max_cycle_duration can be minutes away -- so live scores/status can sit stale far longer than a user would expect from a "live" ticker. Fix: - Restored _tick_plugin_updates_for_vegas() in display_controller.py, wired as the Vegas coordinator's update callback in place of the plain _tick_plugin_updates(). Diffs plugin_last_update before/after the tick and calls vegas_coordinator.mark_plugin_updated(plugin_id) for each plugin that actually got new data (rather than returning the list, since the callback interface no longer consumes a return value). - Restored the has_pending_updates_for_visible_segments() check in render_pipeline.should_recompose(), positioned after (not instead of) the sync-mode early return PR #330 added, so standalone installations regain immediate refresh while synced leader/follower pairs correctly keep deferring hot-swaps to cycle boundaries as PR #330 intended. Test plan: - Added test_display_controller_vegas_tick.py and test_vegas_render_pipeline_recompose.py -- neither area had any prior test coverage, which is very likely why this regression went unnoticed for ~2.5 months. - Verified both new test files fail against the pre-fix code (swapped in the current main versions of both files) with exactly the expected errors -- AttributeError for the deleted method, and the recompose assertion returning False instead of True -- then pass against the fix. - Confirmed the sync-mode deferral this restoration must not break still holds: test_sync_active_defers_pending_updates_to_cycle_boundary. - Full related suite (test_vegas_plugin_adapter, test_vegas_config, test_display_controller_plugin_toggle, test_display_controller_ optimizations, test_plugin_system): 108 passed, 1 pre-existing failure unrelated to this change (test_circuit_breaker, stale mock signature). - Full CI plugin-safety suite (test_harness, test_visual_rendering, test_plugin_matrix): 52 passed, 2 pre-existing skips. * fix(vegas): lock plugin_last_update snapshot/diff against concurrent mutation _tick_plugin_updates_for_vegas() snapshotted and later re-iterated plugin_manager.plugin_last_update from the Vegas background update-tick thread while the main render loop (or other callers) could mutate the same dict concurrently — a real race (unprotected dict iteration/mutation across threads), not just a style nit. Move the snapshot/update/diff into a new locked PluginManager.run_scheduled_updates_with_changes() so all reads and mutations of plugin_last_update happen under one lock, and update DisplayController to use it. The lock is only held around the dict accesses, not the update pass itself, so slow plugin update() calls don't serialize against other callers. Also add a regression test covering that the Vegas coordinator is wired to the Vegas-aware tick callback rather than the plain one. Skipped as not worth the change: - Narrowing the broad `except Exception` around vc.mark_plugin_updated(plugin_id) to specific types: it's a deliberate per-plugin isolation boundary (matches the same pattern used elsewhere in this file for plugin/coordinator calls) and there's no documented, stable set of exceptions that call can raise to narrow to. - Adding an inactive-DisplaySyncManager test to test_vegas_render_pipeline_recompose.py: verified VegasModeCoordinator.set_sync_manager() already normalizes a SyncRole.STANDALONE manager to None before handing it to the render pipeline (src/vegas_mode/coordinator.py:152-156), so should_recompose()'s `is not None` check is correct in practice; the suggested case is already covered by that normalization. --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-12 10:52:18 -04:00
3 people	7f7f0d6464	feat: adaptive layout system — size-aware regions, crisp font ladders, image fitting (#393) ... * feat(layout): adaptive layout & font scaling system for plugins Add src/adaptive_layout.py — opt-in core helpers so plugins render legibly on any panel size without hand-tuned per-display layouts: - Region: integer rect algebra (bands/columns/weighted splits/centering) that partitions space so text bands can't overlap by construction - Font ladders: ordered (family, size) steps known to render crisply (LADDER_GRID: X11 BDFs at native sizes; LADDER_ARCADE: PressStart2P at 8px multiples) — fitting walks the ladder instead of scaling pixel fonts fractionally - LayoutContext: breakpoint tiers, geometry scale vs. a declared design size, and cached fit_text/fit_lines/font_for_rows queries Generalizes the three patterns proven in the field: f1-scoreboard's scale factor, masters-tournament's tiers, baseball-scoreboard's font fallback ladder. Wiring: BasePlugin gains a lazy .layout property and draw_fit(); FontManager gains get_native_bdf_size() and a cache_generation counter; manifest schema gains display.design_size and requires.display_size max_width/max_height; 96x48 joins DEFAULT_TEST_SIZES; the bounds-check harness records negative-coordinate draws; TextHelper's broken measurement helpers are fixed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(layout): adaptive image fitting + composite region helpers Add src/adaptive_images.py — the image counterpart to fit_text: - fit_image(img, box, mode=contain|cover|fill_height|stretch, crop_to_ink, anchor, resample, upscale) promoting the proven plugin patterns (football's crop-to-ink fill-height logos, masters' cover crop + NEAREST flags, static-image's letterbox). Upscales by default — thumbnail()'s downscale-only behavior is why imagery stays tiny on big panels. - draw_fitted_image() pastes aligned within a Region with alpha mask. - One central Pillow>=9.1 RESAMPLE shim replacing ~15 plugin copies. LayoutContext.fit_image() caches results per (identity, box size, options) with a 64-entry LRU; id()-keyed entries pin the source image. BasePlugin.draw_image() is the one-liner adoption path beside draw_fit. Composites in adaptive_layout.py: Region.offset() (user x/y-offset passthrough), scoreboard_regions() (the two-logos-plus-score card math duplicated across six sports plugins, logo_slot = min(H, W//2)), and media_row() (art-left/text-right). Fix LogoHelper's size-blind cache key (stale sizes on panel change); deprecation note on dead image_utils.py. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(harness): scale-up fill check, config variants, multi-size dev gallery Quality gates for adaptive layout: - fill_metrics()/check_scale_up() in the safety harness: overflow catches content too big for a panel, but nothing caught content that stays tiny on panels >= 2x the plugin's declared design size. The check measures lit-content extents and warns (or fails, when a plugin opts into "fill_check": "strict" in test/harness.json) below 50% coverage on the doubled axis. Warn-only by default so no existing plugin breaks. - harness.json "variants": extra runs with config overlays and their own golden dirs, so an opt-in mode (e.g. layout_mode: adaptive) is golden- tested beside the classic default. check_plugin.py loops base + variants and labels variant results mode@name. - Dev preview server: GET /api/sizes (harness size sample), POST /api/render-matrix (render at up to 12 sizes in one call), size-preset dropdown, and an "All Sizes" side-by-side gallery in the preview UI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(plugins): adaptive-lib discoverability + advisory version compat warning Discoverability: re-export the adaptive layout/image API from src.common (the blessed-helpers package plugin authors already know) — canonical paths stay src.adaptive_layout / src.adaptive_images so nothing breaks. Document it in src/common/README.md and cross-link ADAPTIVE_LAYOUT.md from the developer docs authors actually read (quick reference, API reference, advanced dev, font manager, dev preview, plugin dev guide); ADAPTIVE_LAYOUT.md gains adaptive-images, composite-layouts and preserving-user-customization sections. Compat: PluginLoader now logs one advisory warning (never raises) when a plugin's manifest declares a min LEDMatrix version newer than the running core, checking the min_ledmatrix_version / requires.* / versions[] spellings found in the wild. Guarded against stale core version numbers. src/__init__.py __version__ bumped 1.0.0 -> 3.1.0 to match the latest release tag (v3.1.0) — it had never been updated and the compat check needs a truthful number. NOTE: verify this matches the intended release numbering before the next tag. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(layout): add measure_font_crispness — verify a ladder rung isn't blurry PIL antialiases TTF outlines by default; a 'pixel-style' font only rasterizes without antialiasing at specific sizes (for PressStart2P: exact multiples of its 8px design grid). A ladder rung at an unverified size silently renders blurry on an LED panel — this exact bug shipped in both text-display's and football-scoreboard's custom TTF ladders (non-8-multiple PressStart2P sizes, and '5by7.regular'/'4x6-font' at sizes that were never actually crisp). measure_font_crispness(font, sample_text) renders the sample and reports the fraction of ink-bbox pixels that are neither pure black nor pure white. BDF fonts (real bitmaps) always score 0.0; TTF ladders should be verified against this before shipping — see the new TestFontFitting::test_ladder_arcade_is_crisp pattern. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(layout): add fit_text_proportional — proportional sizing vs. always-maximize fit_text always picks the largest ladder rung that fits its box. That's right when an element owns dedicated space, but wrong when several independently-fitted elements need to stay visually harmonious as the panel grows: a score's box might have generous room while a neighboring logo scales by a fixed geometry factor via px() — fit_text lets the score balloon out of proportion (even overlapping the logo) even though its individual pick is technically correct. fit_text_proportional(text, box, base_size_px, ladder) instead targets base_size_px * self.scale (the same scale factor px() already uses), picking the nearest ladder rung at or below that target, still capped to what fits the box, floored at the smallest rung when the target is below every rung. Refactored the shared largest-that-fits/ellipsize walk into _walk_ladder() so fit_text and fit_text_proportional don't duplicate it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(layout): fit_text_proportional gains an axis-specific scale override self.scale (min(width_ratio, height_ratio)) is the right conservative default for anything whose aspect ratio matters, but a caller whose surrounding composition already scales along a single axis — e.g. football-scoreboard's logo_slot = min(height, width // 2), which tracks height alone — needs text sized the same way, or it reads as under-scaled next to logos that grew on a panel that only got taller (128x32 -> 128x64: self.scale stays 1.0 since width didn't grow, but logos still double). fit_text_proportional(..., scale=None) now accepts an explicit override; None keeps the existing self.scale default. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(layout): scoreboard_regions reserves real center space at 2:1 aspect ratios logo_slot = min(height, width // 2) has a blind spot: at exactly 2:1 aspect ratio (width == 2 * height -- a very common shape: two, four, or more square modules stacked into a taller panel) width // 2 and height are equal, so the two logo slots claim the ENTIRE width and leave zero pixels for a center column, no matter how large the panel gets. Not a 'small panel' problem -- 96x48, 128x64, and 256x128 (all exactly 2:1) hit it identically, while the 128x32 design baseline and panels like 192x48 or 256x32 never do, because height is already the tighter constraint there. Two new parameters fix it in the one shared helper every scoreboard-style plugin composes through: - min_center_fraction / min_center_design_px reserve at least max(width * fraction, design_px * ctx.scale) for the center column, capping logo_slot further when needed. The scaled design-px term matters on small panels where a flat fraction alone reserves too little absolute space. - score_bleed_fraction extends the score's own fit box (not the logo slots themselves) a controlled amount into each side -- the same way real broadcast scoreboards let a big score number's edges cross into the team marks flanking it. Without this the reserve alone can still be too narrow for a short score to render without truncating. score_area is now genuinely narrower than the full card width (previously identical to status_band/detail_band, which still span the full width and overlay the logos -- short text there was never the problem). Verified against the full harness size spread: a real game score like '17-21' never needs ellipsis at any tested 2:1-or-tighter aspect ratio (test_score_never_needs_ellipsis_for_a_short_score), and wide panels (128x32/192x48/256x32-style) are provably unaffected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: document scoreboard_regions' center-reserve and score-bleed params Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: address CodeRabbit review on PR #393 - docs: scope the self.layout note to BasePlugin subclasses (others build a LayoutContext directly) and make explicit that adaptive layout is opt-in — classic rendering stays unless a plugin adopts the APIs. - dev_server: broaden the render-request catch (a bad manifest.json now returns a clean 400 instead of an unhandled 500) and stop echoing raw exception text in the loader-failure responses — full tracebacks go to the dev server's console log instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam * fix(dev-server): allowlist plugin_id before any path lookup CodeQL (py/path-injection): plugin_id arrives in request input and flows into filesystem paths via find_plugin_dir. Gate it with the same ^[a-zA-Z0-9_-]{1,64}$ allowlist the web UI's pages_v3 uses, at the single choke point every route resolves through. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam * fix(dev-server): lexical containment check on resolved plugin dirs CodeQL doesn't recognize the interprocedural allowlist as a path-injection barrier; add the canonical one — normalize (without following symlinks, since dev plugins are commonly symlinked into plugins/) and require the result to stay inside the search dir. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam * fix(dev-server): inline normpath containment barrier before render CodeQL doesn't credit the sanitization inside find_plugin_dir along this flow; apply its documented barrier (normpath + startswith against the allowed roots) inline in _parse_render_request, on the exact path that reaches the render/load sinks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam * fix(dev-server): derive plugin dir from trusted directory listings CodeQL's barrier-guard recognition doesn't see a startswith check inside an any() comprehension, so the normalize-and-prefix approach still flagged. Break the taint outright instead: after lookup, re-derive the directory by enumerating the search dirs (iterdir) and matching by path equality — the Path used for all downstream file access is built solely from trusted listings, never from request input. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam * fix(dev-server): use os.scandir for path-injection barrier, redact stack traces from render responses CodeQL doesn't model Path.iterdir() as a taint-clearing enumeration the way it does os.scandir() -- _trusted_plugin_dir's iterdir-based rebuild still traced plugin_id through to the manifest.json open(). Switched to scandir, matching the pattern already verified clean on PR #396. Also stops surfacing raw exception text (update()/display() failures) in the JSON render response -- logs full detail server-side via exc_info instead, returning only the exception class name to the client. And drops path values from three plugin_loader debug/error logs that CodeQL flags as clear-text-logging of externally-influenced data, keeping plugin_id (not flagged) for context. * fix(dev-server): remove conditional-reassignment ambiguity in plugin_dir resolution CodeQL's path-injection flow still traced through _parse_render_request after the scandir fix -- the tainted find_plugin_dir() result and the scandir-derived _trusted_plugin_dir() result shared the same variable name (plugin_dir), reassigned only on the truthy branch. That merge point apparently isn't treated as a barrier by the flow analysis, so it kept tracing the pre-reassignment value through to the manifest open(). Split into two distinct names -- candidate_dir (tainted, used only to call _trusted_plugin_dir) and trusted_dir (the only name used for any downstream file access) -- so there's no reassigned variable for the flow to walk through. * fix: remove unused imports flagged by Codacy Union in adaptive_images.py and field in adaptive_layout.py are both imported but never used -- the last two Codacy findings on this PR, matching the same fix already applied on PR #396. * fix(layout): bound the fit cache; never alias the source image in fits Two latent issues found in a self-review pass: - LayoutContext._fit_cache was an unbounded dict (the image cache got an LRU cap, the text-fit cache didn't). Cache keys embed the fitted TEXT, so a plugin fitting changing strings — a live game clock, a ticker — on a 24/7 service grows it forever. Now LRU-bounded at 512 entries via the same pattern as the image cache. - fit_image returned the caller's ORIGINAL image object when the source was already RGBA at target size (contain/fill_height, no ink crop). ImageFitResult is documented as an independent copy, and LayoutContext caches results — an aliased image lets later mutations of the source corrupt cached fits (or vice versa). Copy in that branch. Both covered by new regression tests. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FqzC1nzTWL4kaqgMaQZFam --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>	2026-07-12 10:38:52 -04:00
ChuckandClaude Sonnet 5	05e7c43b27	fix(plugins): replace dependency marker files with a real satisfaction check (#390) ... * fix(plugins): replace dependency marker files with a real satisfaction check The .dependencies_installed hash-marker system only tracked "was this exact requirements.txt hashed before" — not whether the packages it names are actually present. That made it fragile (a wiped venv, a manually removed package, or a lost/corrupted marker forces a needless full pip reinstall or, worse, a false skip) and produced dead weight for the ~10 plugins whose requirements.txt is comment-only (they still paid a pip subprocess on first boot before a marker existed). Replace it with requirements_are_satisfied() in plugin_loader.py, which checks each real requirement line against importlib.metadata directly, so install_dependencies() only shells out to pip when something is actually missing or version-mismatched. Drops the marker file entirely: removed all marker read/write sites in plugin_loader.py and store_manager.py, the now-pointless marker-cleanup step in the git-update path, the unused legacy marker implementation in plugin_manager.py, and the already-stale clear_dependency_markers.sh script. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ * fix(security): close path-injection gap in dependency-satisfaction checks CodeQL flagged 2 new high-severity "uncontrolled data used in path expression" alerts at the open() calls inside this PR's new requirements_has_real_deps()/requirements_are_satisfied() -- both are reachable from paths that were never run through the basename+trusted-base sanitiser this codebase already uses elsewhere: - PluginLoader.install_dependencies() only applied that sanitiser when its optional plugins_dir argument was actually passed; the "no plugins_dir" branch trusted plugin_dir_real directly. Made plugins_dir required (not Optional) so that branch can't exist, and added an explicit guard in load_plugin() so install_deps=True without a plugins_dir fails loudly instead of silently. Production's only real caller (PluginManager) always passes plugins_dir already; the harness/dev-server/render-plugin callers all use install_deps=False and are unaffected. - StoreManager._install_dependencies() never sanitised plugin_path at all, and its call sites ultimately derive that path from a plugin's own manifest.json "id" field (install_plugin_from_url) -- a malicious plugin could otherwise point requirements_file outside plugins_dir. Applied the same os.path.basename()-based containment pattern PluginLoader already uses (and that CodeQL recognises as a real sanitiser). Added test_install_dependencies_requires_plugins_dir and test_install_dependencies_rejects_path_outside_plugins_dir to lock in the actual security property, not just quiet the scanner. Verified: all 20 tests in test_plugin_loader.py pass, plus the PR's existing test plan (test_plugin_system.py, test_store_manager_caches.py: 53 passed) and the full CI plugin-safety suite (test_harness.py, test_visual_rendering.py, test_plugin_matrix.py: 52 passed, 2 pre-existing skips) all still pass. * fix(security): replace basename-only sanitiser with a trusted-enumeration check The previous commit's os.path.basename() + os.path.join() pattern (which a pre-existing code comment claimed CodeQL recognises as a sanitiser) did not actually clear the alert -- the next CodeQL run still flagged the same 2 sink lines, plus a new one at the os.path.join() call itself. Taking a substring of tainted data apparently isn't treated as a barrier by this query, whatever the comment assumed. Replaced it with find_trusted_subdir(): enumerate the trusted plugins_dir via os.scandir() and only use a name that scandir itself produced, matched by equality against the caller's requested name. The path is then built from that enumerated entry, not from the caller's string -- a value sourced from iterating a trusted, non-tainted directory carries no taint regardless of what it happens to equal, which is a stronger and more conventional allowlist-style barrier than string-stripping. Applied identically in both PluginLoader.install_dependencies() and StoreManager._install_dependencies(), sharing one implementation. Re-verified: all 65 tests across test_plugin_loader.py (20, including the 2 new security regression tests), test_store_manager_caches.py (35), test_plugin_system.py (10) pass, plus the full CI plugin-safety suite (test_harness.py/test_visual_rendering.py/test_plugin_matrix.py: 52 passed, 2 pre-existing skips). * fix(security): redact URL credentials from pip subprocess output before logging CodeQL flagged 3 clear-text-logging-of-secrets alerts in install_requirements_file() (src/common/permission_utils.py:353,360,371). Pre-existing on main, unrelated to this PR's own diff, but now visible since the path-injection alerts that previously took priority in the annotation list are fixed. The underlying risk is real: pip can echo a private index URL's embedded basic-auth credentials (from a requirements.txt --index-url line or PIP_INDEX_URL) back verbatim in its own stderr/stdout on failure, and this function both logs that output directly and returns it to callers -- store_manager.py's _install_dependencies() logs result.stderr from this same function too. Added _redact_url_credentials(), applied immediately after each of the two subprocess.run() calls (mutating result.stderr/stdout in place) rather than patching each log call site individually. This closes the leak at the source: every downstream use -- the three flagged log lines, the "note" string embedded in the returned stdout, and store_manager.py's own logging of the returned result -- gets the redacted text for free. Verified the fixed-phrase "denied" check (`"a password is required" in result.stderr`) is unaffected, since URL syntax and those phrases don't overlap -- covered explicitly by test_does_not_touch_denied_check_phrases. Added test/test_permission_utils.py (6 tests) covering the redaction helper directly and both subprocess.run() call sites (the sudo-wrapper branch, which this repo's scripts/fix_perms/safe_pip_install.sh makes live, and the no-wrapper fallback branch). All pass. * fix(security): stop interpolating req_file/pip-output into log calls The previous commit's redaction (mutating result.stderr/stdout right after each subprocess.run()) didn't clear CodeQL's clear-text-logging alerts -- same lesson as the path-injection fix earlier in this PR: a static analyzer can't tell "this value was already sanitised two lines up" from "this is still the raw tainted value" just by looking at a single log call in isolation, so it conservatively keeps flagging it regardless of what the redaction function actually does. Removed all dynamic interpolation (req_file, result.stderr) from the 3 flagged logger.warning() calls entirely, replacing them with fixed messages plus (for the one that had it) result.returncode, which is a plain int with no possible taint. The full redacted detail is still available where it actually matters -- in the returned CompletedProcess.stderr/stdout and the "note" text -- just not duplicated into a log line a scanner has to reason about in isolation. Re-verified: all 6 test_permission_utils.py tests still pass (they assert on the returned result, not log call arguments), plus the full test_plugin_loader.py/test_store_manager_caches.py/test_plugin_system.py suite (71 passed, 1 pre-existing deselect, 4 subtests). --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>	2026-07-11 08:55:50 -04:00
Chuck	2ffc57cf40	fix(plugin-harness): add no-op process_deferred_updates to test double (#391) ... The safety harness's VisualTestDisplayManager (base of BoundsCheckingDisplayManager) doesn't implement process_deferred_updates(), which 5 first-party ledmatrix-plugins call unconditionally between set_scrolling_state() and their scroll-position update: news, odds-ticker, ledmatrix-leaderboard, stock-news, and ledmatrix-stocks. Any of them fails the harness with AttributeError the moment it's touched (surfaced when ledmatrix-plugins#177 had to add a local hasattr guard in ledmatrix-stocks just to pass CI). Add the method as a no-op, mirroring the existing "no-op for testing" pattern already used for set_scrolling_state, so these plugins render under the harness without every touching PR needing its own guard.	2026-07-11 08:55:03 -04:00
Chuck	aab0e9ade0	fix(plugin-manager): fix TypeError breaking every plugin's scheduled update (#392) ... run_scheduled_updates()'s resource-monitor branch wrapped the update call in a closure stored as a *class* attribute on a dynamically-built type (type('obj', (object,), {'update': monitored_update})()). The descriptor protocol turns a function found via class-attribute lookup into a bound method on instance access, silently prepending the synthetic instance as an implicit first argument -- but monitored_update() takes none, so every call raised "monitored_update() takes 0 positional arguments but 1 was given", was caught by run_scheduled_updates' try/except, and recorded as an update failure. self.resource_monitor is None by default and was dormant until PR #388 ("activate dormant plugin health/metrics subsystem") wired it up in both display_controller.py and web_interface/app.py -- meaning this bug went live in every real deployment as of that merge (2026-07-09) despite the buggy line itself dating back to 2025-12-27. In practice this means no plugin's update() has succeeded since upgrading past #388: circuit breakers cycle through half-open -> immediate failure -> reopened every health-check interval forever, and all plugin data (scores, odds, prices, etc.) goes stale from whatever was last fetched before the upgrade. Confirmed live on a running instance: odds-ticker (and stock-news, ledmatrix-stocks, baseball-scoreboard, ledmatrix-leaderboard, of-the-day) failing this exact way every 5-minute circuit-breaker retry. Fixed by using types.SimpleNamespace(update=monitored_update) instead of a dynamic class: SimpleNamespace stores attributes on the instance itself, so attribute lookup returns the plain function unchanged -- never routed through the class-attribute descriptor protocol that injects an implicit self. Added test_run_scheduled_updates_calls_update_with_resource_monitor to test/test_plugin_system.py using a real PluginResourceMonitor (not a mock of it), so the test exercises the actual descriptor-binding behavior that caused this. Verified the test fails with the exact reported error against the pre-fix code and passes against the fix.	2026-07-11 08:54:30 -04:00
ChuckandClaude Opus 4.8	3b93024993	feat: activate dormant plugin health/metrics subsystem and surface it in the web UI (#388) ... * feat(plugin-system): activate dormant plugin health & metrics subsystem PluginManager shipped a fully-built health tracker, resource monitor and circuit breaker that were never instantiated (health_tracker/resource_monitor were left as None), so the circuit breaker never engaged and the existing health/metrics API routes always returned "not available". - DisplayController now wires a PluginHealthTracker and PluginResourceMonitor onto the plugin manager, enabling the circuit breaker (a repeatedly-failing plugin's update() is skipped after consecutive failures, then retried after a cooldown) and per-plugin execution-time metrics. Both persist to the shared cache. - load_plugin() now validates each plugin's config against its JSON schema in a strictly warn/degrade-only way: a violation logs a warning and flags the plugin degraded in the health tracker, but never changes whether the plugin loads or its pass/fail behaviour. Adds PluginHealthTracker.set_degraded(), which never touches the circuit breaker. - ResourceMonitor CPU/memory sampling now reuses a cached psutil.Process and reads cpu_percent(interval=None), so monitoring no longer blocks ~100ms per call on the display loop's update path. - Fix DiskCache.get() raising TypeError for max_age=None ("never expires"), which silently discarded persisted plugin health/metrics on read and thus broke cross-process and post-restart surfacing. - Fix two dead PluginManager helpers that called non-existent tracker methods. Tests: new test_resource_monitor, test_plugin_health, test_plugin_manager_schema_soft; extended test_cache_manager and test_display_controller. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UvTav268UXv44ub9K11LYq * feat(web-ui): surface plugin health, metrics and load state With the health/metrics subsystem now active in the display service, expose it in the web UI (which runs as a separate process from the display loop): - Wire a health tracker / resource monitor backed by the shared on-disk cache into the web process so /api/v3/plugins/health and /plugins/metrics read the data the display service persists. - Build those route responses per installed plugin id (the tracker's in-memory view is empty in a fresh web process) so cross-process data is included. - Add state + error_info to /plugins/installed entries so the UI can show why a plugin isn't running instead of just loaded:false. - Add a "Plugin Health" panel to the Tools page (circuit status, avg/max update time, update count, last error) plus PluginAPI.getPluginMetrics(). Tests: route-level tests for the health/metrics endpoints in test_web_api. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UvTav268UXv44ub9K11LYq * fix(plugin-metrics): refresh cross-process health/metrics reads; type hints Addresses CodeRabbit review on #388: - Major: the web process's health/resource trackers cached the first persisted read in an in-memory dict (and the CacheManager memory tier held max_age=None entries indefinitely), so a long-lived web process showed the first snapshot and never reflected the display service's later updates. Add an opt-in force_reload path (get_health_summary/get_health_state/_load_health_state and get_metrics_summary/get_metrics) that bypasses the in-memory copy and, via a new memory_ttl passthrough on CacheManager.get, the cache manager's memory tier — so each /plugins/health and /plugins/metrics poll reads fresh persisted state. Default behaviour (force_reload=False) is unchanged for the display process and existing callers. - Minor: DiskCache.get type hint is now Optional[int] with the None ("never expires") semantics documented, matching MemoryCache.get. Tests: new force_reload staleness cases in test_plugin_health and test_resource_monitor. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UvTav268UXv44ub9K11LYq --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-09 07:54:18 -04:00
ChuckandClaude Sonnet 5	85d321cf33	fix: plugin_loader retries with --ignore-installed on apt/pip RECORD conflicts (#386) ... * fix: plugin_loader retries with --ignore-installed before assuming apt package satisfies pin install_dependencies treated any "uninstall-no-record-file" pip failure as "dependency satisfied" and wrote the success marker without ever attempting --ignore-installed, unlike install_dependencies_apt.py and safe_pip_install.sh (added in #385 for the Plugin Store/first-time-install paths). A plugin pinning a newer version of a system-managed package (e.g. requests) would silently keep running against whatever version apt shipped, while the marker file claimed the pinned requirement was met. Now retries the same install with --ignore-installed on that specific failure so pip actually lays the pinned version down (shadowing the system-managed copy) before falling back to the prior tolerant behavior if the retry itself fails too. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx * chore: suppress Codacy finding on new retry subprocess.run call Same generic Bandit/semgrep pattern-match on non-literal subprocess.run argv flagged in #385's install_requirements_file, now on the new --ignore-installed retry call added here: list-form argv (no shell=True), sys.executable is this process's own interpreter, and requirements_file is built internally by find_plugin_directory, never raw external input. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx * fix: tolerate a timed-out --ignore-installed retry, dedupe marker-write logic CodeRabbit review caught a real inconsistency: if the --ignore-installed retry itself timed out, subprocess.TimeoutExpired propagated to the outer handler and returned False, failing plugin load — contradicting the intended "tolerate this specific apt/pip conflict" behavior, where a mere non-zero retry return code already returns True. Wraps the retry in its own try/except so a timeout is logged and tolerated the same way as any other retry failure. Also extracts the marker-writing logic (open/write/chmod, ignoring OSError) into _write_dependency_marker, since it was duplicated identically between the direct-success path and the apt-conflict-retry-fallback path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx * fix: revert marker-write dedup helper, resolves CodeQL path-injection alert CodeQL flagged _write_dependency_marker's open(marker_file, ...) as "uncontrolled data used in path expression" (high severity) once the marker-write logic was extracted into its own method. marker_file is actually safe — it's built from safe_plugin_dir, which install_dependencies sanitizes via os.path.basename() (CodeQL's own recognized py/path-injection sanitizer, per the existing comment a few lines above) — but CodeQL's interprocedural analysis doesn't carry that sanitized status across the new method boundary, since the sanitizer call and the open() sink were no longer in the same function. This exact code produced zero CodeQL findings before the extraction (in two duplicated inline blocks) and is unchanged in what data reaches it — only its location moved. Reverting the extraction (keeping CodeRabbit's other, independent timeout-handling fix) restores the previously-clean shape rather than trying to convince the analyzer's cross-function taint tracking that a refactor changed nothing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-08 11:19:37 -04:00
ChuckandClaude Sonnet 5	63a233f3ed	fix: dependency installation gaps in Plugin Store and first-time install (#385) ... * fix: install plugin dependencies through root-visible installer in Plugin Store install_plugin/update_plugin (store_manager.py) installed requirements.txt with a bare `pip3` off PATH, bypassing the root-visible installer added in #380 for the "Reinstall Plugin Deps" button. Two bugs stacked: (1) `pip3` can resolve to a different Python install than the one that actually runs ledmatrix.service, and (2) even when it resolves correctly, ledmatrix-web runs as a non-root user so the package lands in that user's local site-packages, invisible to root-run ledmatrix.service. Either way the install reports success and writes the .dependencies_installed hash marker, so plugin_loader's own (correct) install-on-load path skips reinstalling — leaving the dependency permanently missing until a user finds and clicks the separate "Reinstall Plugin Deps" tool. This is why users kept hitting "No module named 'astral'" for the weather plugin even after installing it from the Store. Extracts the sudo-wrapper-then-fallback install logic from api_v3.py's _pip_install_requirements into src/common/permission_utils.py as install_requirements_file, and routes store_manager.py's dependency installation through it so the automatic Store install/update path now matches the manual "Reinstall Plugin Deps" path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx * chore: suppress Codacy false-positive on subprocess.run in install_requirements_file Codacy's generic subprocess-security rule (Bandit B603 equivalent) flagged the pip/sudo subprocess.run calls in install_requirements_file for lacking a "static string argument" — the standard pattern-based flag for any subprocess.run() call with a variable in its argv list. Both calls use list-form argv (no shell=True, so no shell-injection surface), and the only dynamic value is req_file, a Path built internally by callers rather than raw external input; safe_pip_install.sh independently re-validates it before installing anything as root. Suppresses with inline `# nosec B603` comments matching this codebase's existing convention (see permission_utils.py's own PROTECTED_SYSTEM_DIRECTORIES, display_manager.py, sync_manager.py, etc.). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx * fix: first-time install script fails on apt-managed requests package web_interface/requirements.txt and requirements.txt both pin requests>=2.33.0,<3.0.0, but Raspberry Pi OS ships an apt-managed python3-requests with no pip RECORD file. Upgrading it via plain `pip install` aborts with "uninstall-no-record-file" because pip refuses to uninstall a package it has no record of, in place — which is exactly the "Some web interface dependencies failed to install" warning first-time install hits. scripts/install_dependencies_apt.py and scripts/fix_perms/safe_pip_install.sh already work around this with --ignore-installed (lets pip lay the new version down in /usr/local, shadowing the apt copy, instead of trying to remove it first). first_time_install.sh's own direct pip invocations — the per-package requirements.txt loop, the web_interface/requirements.txt install, and the requirements_web_v2.txt fallback — didn't have it. Adds --ignore-installed to all three so first-time install no longer fails on this well-known apt/pip conflict. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx * chore: add nosemgrep to subprocess.run calls Codacy still flagged The prior # nosec B603 comments suppressed Bandit's check but Codacy's semgrep-based rule ("subprocess function 'run' without a static string") kept flagging the same two lines as a critical security issue even after that fix landed. install_dependencies_apt.py's _run() already needed both tags together (# nosec B603 B607 ... # nosemgrep) for the identical subprocess.run pattern, so apply the same double suppression here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx * fix: add --ignore-installed to install_requirements_file fallback path CodeRabbit review caught this (confirming a gap already flagged in conversation): the non-sudo fallback pip install in install_requirements_file was missing --ignore-installed, unlike the sudo-wrapper branch and safe_pip_install.sh. Without it, the same apt/pip RECORD-file conflict this PR fixes elsewhere (first_time_install.sh, install_dependencies_apt.py) could still hit installs that fall back to this path (e.g. a plugin's requirements.txt on a host where safe_pip_install.sh isn't set up yet). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X1NnDduw53kTe67i5zWwYx --------- Co-authored-by: Claude <noreply@anthropic.com>	2026-07-08 09:59:12 -04:00
Ron PierceandClaude Opus 4.8	d22d0a3754	fix(plugins): stop core updates from resurrecting uninstalled built-in plugins (#368) ... * fix(plugins): stop core updates from resurrecting uninstalled built-in plugins Built-in plugins (e.g. web-ui-info, starlark-apps) are committed into the repo under plugin-repos/. When a user uninstalls one, a subsequent core `git pull` update restores the committed files, so the plugin reappears on every update. The update endpoint stashes the deletion and never pops it, and `git pull` faithfully restores any committed file whose deletion was never committed — so excluding plugin-repos/ from the stash can't fix this (it would only make `git pull --rebase` fail on a dirty tree). Add a persistent uninstall registry (config/uninstalled_plugins.json, gitignored) that survives restarts, unlike the existing in-memory tombstone: - Uninstall records the plugin id; install clears it. - purge_uninstalled_plugins() re-removes any recorded plugin whose directory reappears on disk; called after a successful git-pull update and at web startup (covers manual `git pull` on the Pi too). - The state reconciler also refuses to auto-repair a persistently uninstalled plugin. Wires up mark_recently_uninstalled in the uninstall flow (previously only referenced by tests) via the new persistent record. Adds regression tests covering record/forget/purge lifecycle, persistence across manager instances, and corrupt-registry tolerance. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(plugins): validate uninstall-registry ids and lock registry writes Address review feedback on the persistent uninstall registry: - Critical: validate plugin ids on read/record and add a containment guard in purge_uninstalled_plugins. A corrupt or hand-edited registry entry of "" resolves to the plugins root, so purge could have deleted every plugin; traversal ids ("..", "../x") could target paths outside the root. Invalid ids are now dropped on read, refused on record, and never removed unless the path is a direct child of the plugins directory. - Major: guard record/forget read-modify-write with a lock so concurrent install/uninstall requests can't lose updates. - Minor: narrow the startup and post-update purge exception handlers from bare Exception to (OSError, RuntimeError). Adds regression tests for empty-id, traversal-id, and invalid-record cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-06-11 18:18:28 -04:00
Ron PierceandClaude Opus 4.8	313e35a98f	Add cross-size/cross-screen plugin safety harness (#361) ... * feat(testing): add cross-size/cross-screen plugin safety harness Render every plugin across all supported matrix sizes (64x32, 128x32, 128x64, 256x32) and every declared screen, failing on crashes, content drawn past the panel edge, or visual drift vs committed golden images. - BoundsCheckingDisplayManager: oversized-canvas overflow detection - harness.py: multi-size/multi-screen render engine + golden compare - scripts/check_plugin.py: CLI (functional+bounds, --out-dir, --update-golden, --freeze-time); render_plugin.py refactored onto shared loading helpers - test/plugins/test_harness.py + test_plugin_matrix.py (parametrized, honors per-plugin test/harness.json; skips when no plugins present) - MockCacheManager.cache_dir so cache-dir-using plugins load headlessly - .github/workflows/test.yml + docs/plugin-safety-harness.md Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(testing): address PR review feedback on plugin safety harness - check_plugin: friendly error for non-numeric --sizes; reject non-object --config / --mock-data JSON; sanitize plugin mode before using as a filename; stop --update-golden from masking crash/overflow failures - bounds_display_manager: pad the canvas out to the largest supported panel (not a fixed 16px) so far-overshoot coordinates are caught, not clipped - harness: merge config_schema defaults inside render_plugin_matrix; surface update() failures as a non-fatal warning + result field instead of a debug log; sanitize mode in golden_path - loading: fail fast when harness.json references a missing mock_data fixture - mocks: clean up the per-instance temp cache dir via weakref.finalize - test_plugin_matrix: add a discovery guard that fails when LEDMATRIX_REQUIRE_PLUGINS=1 but none found (still skips locally); type hints - bound test deps with upper version pins for deterministic CI Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(testing): render plugins across arbitrary panel sizes, not a fixed list Addresses maintainer feedback that there is no canonical set of supported panel sizes — a build can be any size/configuration (square, 2x2, 4x4, 8x2, long strips, tall stacks). - sizes.py: SUPPORTED_SIZES -> DEFAULT_TEST_SIZES (back-compat alias kept), reframed as a representative SAMPLE of real panel-grid arrangements rather than an authoritative list; add parse_size_token / coerce_sizes / resolve_test_sizes helpers - sizes are now fully overridable: LEDMATRIX_TEST_SIZES env (global, e.g. test on your exact hardware) > per-plugin harness.json "sizes" > default sample; CLI --sizes unchanged - bounds_display_manager: pad the canvas to the largest panel IN THE CURRENT RUN (via overflow_extent) instead of a hardcoded max, so cross-size overflow detection scales to whatever sizes a run uses - harness: compute per-run extent and thread it into the bounds manager - tests: arbitrary-shape + size-parsing/precedence coverage - docs: rewrite "Supported sizes" -> "Sizes: a sample, not a fixed list" Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(testing): fail the harness on non-connectivity update() errors Addresses the remaining review thread: recording every update() exception as a non-fatal warning still let a real update() regression pass green as long as display() survived. Now update() failures are classified — a tolerated set of connectivity errors (ConnectionError/TimeoutError/socket/ssl/urllib/http/ requests) is recorded non-fatally (expected with no network in CI), while any other exception is treated as a genuine bug and fails that render. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci(security): pin actions to SHAs and disable checkout credential persistence Addresses the CodeRabbit/zizmor workflow-hardening finding: pin actions/checkout and actions/setup-python to full commit SHAs and set persist-credentials: false on checkout to reduce supply-chain and token-exposure risk. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(testing): validate positive sizes; narrow requests import except Two review findings: - sizes.py: parse_size_token / coerce_sizes now reject non-positive dimensions (0x32, -64x32) with a clear message instead of passing invalid sizes downstream (CodeRabbit). - harness.py: the optional `requests` import now catches ImportError specifically and logs instead of `except Exception: pass`, clearing the Codacy medium "Try, Except, Pass" (harness.py L52) and Ruff S110/BLE001. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>	2026-06-05 14:32:52 -04:00