From d9683e28be16d509f4ce851336fe81dce7dffb7e Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Thu, 6 Aug 2026 14:04:23 -0400 Subject: [PATCH] Codebase audit: fix shipping bugs, remove verified-dead code, repair doc drift, add regression guards (#438) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 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 + + + @@ -1010,7 +1013,11 @@ - +