docs: refresh and correct stale documentation across repo#306
Merged
ChuckBuilds merged 18 commits intomainfrom Apr 8, 2026
Merged
docs: refresh and correct stale documentation across repo#306ChuckBuilds merged 18 commits intomainfrom
ChuckBuilds merged 18 commits intomainfrom
Conversation
Walked the README and docs/ tree against current code and fixed several
real bugs and many stale references. Highlights:
User-facing
- README.md: web interface install instructions referenced
install_web_service.sh at the repo root, but it actually lives at
scripts/install/install_web_service.sh.
- docs/GETTING_STARTED.md: every web UI port reference said 5050, but
the real server in web_interface/start.py:123 binds 5000. Same bug
was duplicated in docs/TROUBLESHOOTING.md (17 occurrences). Fixed
both.
- docs/GETTING_STARTED.md: rewrote tab-by-tab instructions. The doc
referenced "Plugin Store", "Plugin Management", "Sports Configuration",
"Durations", and "Font Management" tabs - none of which exist. Real
tabs (verified in web_interface/templates/v3/base.html) are: Overview,
General, WiFi, Schedule, Display, Config Editor, Fonts, Logs, Cache,
Operation History, Plugin Manager (+ per-plugin tabs).
- docs/GETTING_STARTED.md: removed references to a "Test Display"
button (doesn't exist) and "Show Now" / "Stop" plugin buttons. Real
controls are "Run On-Demand" / "Stop On-Demand" inside each plugin's
tab (partials/plugin_config.html:792).
- docs/TROUBLESHOOTING.md: removed dead reference to
troubleshoot_weather.sh (doesn't exist anywhere in the repo); weather
is now a plugin in ledmatrix-plugins.
Developer-facing
- docs/PLUGIN_API_REFERENCE.md: documented draw_image() doesn't exist
on DisplayManager. Real plugins paste onto display_manager.image
directly (verified in src/base_classes/{baseball,basketball,football,
hockey}.py). Replaced with the canonical pattern.
- docs/PLUGIN_API_REFERENCE.md: documented cache_manager.delete() doesn't
exist. Real method is clear_cache(key=None). Updated the section.
- docs/PLUGIN_API_REFERENCE.md: added 10 missing BasePlugin methods that
the doc never mentioned: dynamic-duration hooks, live-priority hooks,
and the full Vegas-mode interface.
- docs/PLUGIN_DEVELOPMENT_GUIDE.md: same draw_image fix.
- docs/DEVELOPMENT.md: corrected the "Plugin Submodules" section. Plugins
are NOT git submodules - .gitmodules only contains
rpi-rgb-led-matrix-master. Plugins are installed at runtime into the
plugins directory configured by plugin_system.plugins_directory
(default plugin-repos/). Both internal links in this doc were also
broken (missing relative path adjustment).
- docs/HOW_TO_RUN_TESTS.md: removed pytest-timeout from install line
(not in requirements.txt) and corrected the test/integration/ path
(real integration tests are at test/web_interface/integration/).
Replaced the fictional file structure diagram with the real one.
- docs/EMULATOR_SETUP_GUIDE.md: clone URL was a placeholder; default
pixel_size was documented as 16 but emulator_config.json ships with 5.
Index
- docs/README.md: rewrote. Old index claimed "16-17 files after
consolidation" but docs/ actually has 38 .md files. Four were missing
from the index entirely (CONFIG_DEBUGGING, DEV_PREVIEW,
PLUGIN_ERROR_HANDLING, STARLARK_APPS_GUIDE). Trimmed the navel-gazing
consolidation/statistics sections.
Out of scope but worth flagging:
- src/plugin_system/resource_monitor.py:343 and src/common/api_helper.py:287
call cache_manager.delete(key) but no such method exists on
CacheManager. Both call sites would AttributeError at runtime if hit.
Not fixed in this docs PR - either add a delete() shim or convert
callers to clear_cache().
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WEB_INTERFACE_GUIDE.md - Web UI port: 5050 -> 5000 (4 occurrences) - Tab list was almost entirely fictional. Documented tabs: General Settings, Display Settings, Durations, Sports Configuration, Plugin Management, Plugin Store, Font Management. None of these exist. Real tabs (verified in web_interface/templates/v3/base.html: 935-1000): Overview, General, WiFi, Schedule, Display, Config Editor, Fonts, Logs, Cache, Operation History, plus Plugin Manager and per-plugin tabs in the second nav row. Rewrote the navigation section, the General/Display/Plugin sections, and the Common Tasks walkthroughs to match. - Quick Actions list referenced "Test Display" button (doesn't exist). Replaced with the real button list verified in partials/overview.html:88-152: Start/Stop Display, Restart Display Service, Restart Web Service, Update Code, Reboot, Shutdown. - API endpoints used /api/* paths. The api_v3 blueprint mounts at /api/v3 (web_interface/app.py:144), so the real paths are /api/v3/config/main, /api/v3/system/status, etc. Fixed. - Removed bogus "Sports Configuration tab" walkthrough; sports favorites live inside each scoreboard plugin's own tab now. - Plugin directory listed as /plugins/. Real default is plugin-repos/ (verified in config/config.template.json:130 and display_controller.py:132); plugins/ is a fallback. - Removed "Swipe navigation between tabs" mobile claim (not implemented). WIFI_NETWORK_SETUP.md - 21 occurrences of port 5050 -> 5000. - All /api/wifi/* curl examples used the wrong path. The real wifi API routes are at /api/v3/wifi/* (api_v3.py:6367-6609). Fixed. - ap_password default was documented as "" (empty/open network) but config/wifi_config.json ships with "ledmatrix123". Updated the Quick Start, Configuration table, AP Mode Settings section, and Security Recommendations to match. Also clarified that setting ap_password to "" is the way to make it an open network. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughDocumentation updated across many guides to reflect web UI port changes to :5000, API versioning under /api/v3, a two-row UI (system tabs + per-plugin tabs), plugin storage configurable with default Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
docs/PLUGIN_API_REFERENCE.md (1)
687-701: Duplicate clear_cache() documentation.The
clear_cache()method is documented twice in this file:
- Lines 540-556 (in the "Basic Methods" section with detailed explanation)
- Lines 687-701 (in the "Utility Methods" section with similar content)
This duplication could lead to maintenance issues if the method signature or behavior changes. Consider removing the duplicate or consolidating the documentation.
♻️ Proposed fix to remove duplication
Remove the duplicate section at lines 687-701 and add a cross-reference instead:
-#### `clear_cache(key: Optional[str] = None) -> None` - -Clear cache for a specific key or all keys. - -**Parameters**: -- `key` (str, optional): Specific key to clear. If `None`, clears all cache. - -**Example**: -```python -# Clear specific key -self.cache_manager.clear_cache("weather_data") - -# Clear all cache -self.cache_manager.clear_cache() -``` +> **Note**: See `clear_cache()` in the Basic Methods section above for cache clearing functionality.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/PLUGIN_API_REFERENCE.md` around lines 687 - 701, The docs duplicate the clear_cache() entry; remove the redundant Utility Methods block that documents clear_cache() (the snippet and heading around `clear_cache(key: Optional[str] = None) -> None`) and replace it with a short cross-reference pointing to the original Basic Methods `clear_cache()` documentation, ensuring references mention the `clear_cache` symbol so readers can find the canonical description.docs/GETTING_STARTED.md (3)
313-317: Add language identifier to fenced code block.The WiFi Access Point code block also needs a language identifier.
📝 Suggested fix
-``` -Network Name: LEDMatrix-Setup +```text +Network Name: LEDMatrix-Setup Password: (none - open network) URL when connected: http://192.168.4.1:5000</details> As per coding guidelines, this is flagged by markdownlint-cli2 MD040. <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/GETTING_STARTED.mdaround lines 313 - 317, Add a language identifier to
the fenced code block that contains "Network Name: LEDMatrix-Setup" so it
becomestext ...; locate the fenced block showing the WiFi Access Point
details (the block starting with "Network Name: LEDMatrix-Setup") and update the
opening triple backticks to include the language identifier "text" to satisfy
markdownlint MD040.</details> --- `291-309`: **Add language identifier to fenced code block.** Similar to the earlier code block, this quick reference would benefit from a language identifier for better rendering. <details> <summary>📝 Suggested fix</summary> ```diff -``` -Main Interface: http://your-pi-ip:5000 +```text +Main Interface: http://your-pi-ip:5000 System tabs: - Overview System stats, live preview, quick actions ``` </details> As per coding guidelines, this is flagged by markdownlint-cli2 MD040. <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/GETTING_STARTED.mdaround lines 291 - 309, The fenced code block that
lists the main interface and tabs (starting with "Main Interface:
http://your-pi-ip:5000" and the subsequent "System tabs:" and "Plugin tabs
(second row):" lines) is missing a language identifier; update that
triple-backtick fence to include a language token (e.g., ```text) so the block
is annotated for markdownlint (MD040) and renders consistently.</details> --- `57-59`: **Add language identifier to fenced code block.** The markdownlint tool flags this code block as missing a language specifier. While this is a URL example (not executable code), adding a language identifier improves rendering and syntax highlighting in documentation viewers. <details> <summary>📝 Suggested fix</summary> ```diff -``` -http://your-pi-ip:5000 -``` +```text +http://your-pi-ip:5000 +``` ``` </details> As per coding guidelines, this is flagged by markdownlint-cli2 MD040. <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/GETTING_STARTED.mdaround lines 57 - 59, Add a language identifier to
the fenced code block containing the URL so markdownlint MD040 is satisfied:
update the triple-backtick block around "http://your-pi-ip:5000" to include a
language tag such as "text" (i.e., changetotext) in the
GETTING_STARTED.md fenced block.</details> </blockquote></details> <details> <summary>docs/WEB_INTERFACE_GUIDE.md (1)</summary><blockquote> `216-216`: **Minor markdown style inconsistency.** Markdownlint flags this line for using underscore-style bold (`__text__`) instead of the expected asterisk-style (`**text**`). While this doesn't affect rendering, consistency improves maintainability. To fix this, locate line 216 and replace underscore-style bold with asterisk-style bold: ```diff -__Run On-Demand__ / __Stop On-Demand__ +**Run On-Demand** / **Stop On-Demand** ``` As per coding guidelines, this is flagged by markdownlint-cli2 MD050. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@docs/WEB_INTERFACE_GUIDE.md` at line 216, Replace the underscore-style bold at the flagged location (line referenced in docs/WEB_INTERFACE_GUIDE.md) by changing any occurrences of __...__ to the asterisk-style **...** so the text uses **bold** instead of __bold__ (ensure you only change the bold markers and preserve the inner text and surrounding whitespace/punctuation). ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@docs/GETTING_STARTED.md:
- Line 75: Update the "Columns" line in GETTING_STARTED.md to clarify that 64
and 96 are common/recommended values rather than the only valid options and
include the actual supported range; e.g., change the line that currently reads
"Columns: 64 or 96 (match your hardware)" to indicate "Columns: commonly 64 or
96 (recommended); valid range 16–128 depending on your hardware" so readers know
the full allowed range.
Nitpick comments:
In@docs/GETTING_STARTED.md:
- Around line 313-317: Add a language identifier to the fenced code block that
contains "Network Name: LEDMatrix-Setup" so it becomestext ...; locate
the fenced block showing the WiFi Access Point details (the block starting with
"Network Name: LEDMatrix-Setup") and update the opening triple backticks to
include the language identifier "text" to satisfy markdownlint MD040.- Around line 291-309: The fenced code block that lists the main interface and
tabs (starting with "Main Interface: http://your-pi-ip:5000" and the subsequent
"System tabs:" and "Plugin tabs (second row):" lines) is missing a language
identifier; update that triple-backtick fence to include a language token (e.g.,consistently. - Around line 57-59: Add a language identifier to the fenced code block containing the URL so markdownlint MD040 is satisfied: update the triple-backtick block around "http://your-pi-ip:5000" to include a language tag such as "text" (i.e., change ``` to ```text) in the GETTING_STARTED.md fenced block. In `@docs/PLUGIN_API_REFERENCE.md`: - Around line 687-701: The docs duplicate the clear_cache() entry; remove the redundant Utility Methods block that documents clear_cache() (the snippet and heading around `clear_cache(key: Optional[str] = None) -> None`) and replace it with a short cross-reference pointing to the original Basic Methods `clear_cache()` documentation, ensuring references mention the `clear_cache` symbol so readers can find the canonical description. In `@docs/WEB_INTERFACE_GUIDE.md`: - Line 216: Replace the underscore-style bold at the flagged location (line referenced in docs/WEB_INTERFACE_GUIDE.md) by changing any occurrences of __...__ to the asterisk-style **...** so the text uses **bold** instead of __bold__ (ensure you only change the bold markers and preserve the inner text and surrounding whitespace/punctuation).🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID:
21d12da4-dbe0-4264-8a52-c40298843cf3📒 Files selected for processing (11)
README.mddocs/DEVELOPMENT.mddocs/EMULATOR_SETUP_GUIDE.mddocs/GETTING_STARTED.mddocs/HOW_TO_RUN_TESTS.mddocs/PLUGIN_API_REFERENCE.mddocs/PLUGIN_DEVELOPMENT_GUIDE.mddocs/README.mddocs/TROUBLESHOOTING.mddocs/WEB_INTERFACE_GUIDE.mddocs/WIFI_NETWORK_SETUP.md
REST_API_REFERENCE.md
- Wrong path: /fonts/delete/<font_family> -> /fonts/<font_family>
(verified the real DELETE route in
web_interface/blueprints/api_v3.py).
- Diffed the documented routes against the real api_v3 blueprint
(92 routes vs the 71 documented). Added missing sections:
- Error tracking (/errors/summary, /errors/plugin/<id>, /errors/clear)
- Health (/health)
- Schedule dim/power (/config/dim-schedule GET/POST)
- Plugin-specific endpoints (calendar/list-calendars,
of-the-day/json/upload+delete, plugins/<id>/static/<path>)
- Starlark Apps (12 endpoints: status, install-pixlet, apps CRUD,
repository browse/install, upload)
- Font preview (/fonts/preview)
- Updated table of contents with the new sections.
- Added a footer note that the API blueprint mounts at /api/v3
(app.py:144) and that SSE stream endpoints are defined directly on
the Flask app at app.py:607-615.
ADVANCED_FEATURES.md
- Vegas Scroll Mode section was actually accurate (verified all
config keys match src/vegas_mode/config.py:15-30).
- On-Demand Display section had multiple bugs:
- 5 occurrences of port 5050 -> 5000
- All API paths missing /v3 (e.g. /api/display/on-demand/start
should be /api/v3/display/on-demand/start)
- "Settings -> Plugin Management -> Show Now Button" UI flow doesn't
exist. Real flow: open the plugin's tab in the second nav row,
click Run On-Demand / Stop On-Demand.
- "Python API Methods" section showed
controller.show_on_demand() / clear_on_demand() /
is_on_demand_active() / get_on_demand_info() — none of these
methods exist on DisplayController. The on-demand machinery is
all internal (_set_on_demand_*, _activate_on_demand, etc) and
is driven through the cache_manager. Replaced the section with
a note pointing to the REST API.
- All Use Case Examples used the same fictional Python calls.
Replaced with curl examples against the real API.
- Cache Management section claimed "On-demand display uses Redis cache
keys". LEDMatrix doesn't use Redis — verified with grep that
src/cache_manager.py has no redis import. The cache is file-based,
managed by CacheManager (file at /var/cache/ledmatrix/ or fallback
paths). Rewrote the manual recovery section:
- Removed redis-cli commands
- Replaced cache.delete() Python calls with cache.clear_cache()
(the real public method per the same bug already flagged in
PLUGIN_API_REFERENCE.md)
- Replaced "Settings -> Cache Management" with the real Cache tab
- Documented the actual cache directory candidates
- Background Data Service section:
- Used "nfl_scoreboard" as the plugin id in the example.
The real plugin is "football-scoreboard" (handles both NFL and
NCAA). Fixed.
- "Implementation Status: Phase 1 NFL only / Phase 2 planned"
section was severely outdated. The background service is now
used by all sports scoreboards (football, hockey, baseball,
basketball, soccer, lacrosse, F1, UFC), the odds ticker, and
the leaderboard plugin. Replaced with a current "Plugins using
the background service" note.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
PLUGIN_STORE_GUIDE.md
- 19 occurrences of port 5050 -> 5000
- All API paths missing /v3 (e.g. /api/plugins/install ->
/api/v3/plugins/install). Bulk fix.
PLUGIN_REGISTRY_SETUP_GUIDE.md
- Same port + /api/v3 fixes (3 occurrences each)
- "Go to Plugin Store tab" -> "Open the Plugin Manager tab and scroll
to the Install from GitHub section" (the real flow for registry
setup is the GitHub install section, not the Plugin Store search)
PLUGIN_CONFIG_QUICK_START.md
- Port 5001 -> 5000 (5001 is the dev_server.py default, not the web UI)
- "Plugin Store tab" install flow -> real Plugin Manager + Plugin Store
section + per-plugin tab in second nav row
- Removed reference to PLUGIN_CONFIG_TABS_SUMMARY.md (archived doc)
PLUGIN_CONFIGURATION_TABS.md
- "Plugin Management vs Configuration" section confusingly described
a "Plugins Tab" that doesn't exist as a single thing. Rewrote to
describe the real two-piece structure: Plugin Manager tab (browse,
install, toggle) vs per-plugin tabs (configure individual plugins).
PLUGIN_DEPENDENCY_GUIDE.md
- Port 5001 -> 5000
PLUGIN_DEPENDENCY_TROUBLESHOOTING.md
- Wrong port (8080) and wrong UI nav ("Plugin Store or Plugin
Management"). Fixed to the real flow.
PLUGIN_QUICK_REFERENCE.md
- "Plugin Location: ./plugins/ directory" -> default is plugin-repos/
(verified in config/config.template.json:130 and
display_controller.py:132). plugins/ is a fallback.
- File structure diagram showed plugins/ -> plugin-repos/.
- Web UI install flow: "Plugin Store tab" -> "Plugin Manager tab ->
Plugin Store section". Also fixed Configure ⚙️ button (doesn't
exist) and "Drag and drop reorder" (not implemented).
- API examples: replaced ad-hoc Python pseudocode with real curl
examples against /api/v3/plugins/* endpoints. Pointed at
REST_API_REFERENCE.md for the full list.
- "Migration Path Phase 1-5" was a roadmap written before the plugin
system shipped. The plugin system is now stable and live. Removed
the migration phases as they're history, not a roadmap.
- "Quick Migration" section called scripts/migrate_to_plugins.py
which doesn't exist anywhere in the repo. Removed.
- "Plugin Registry Structure" referenced
ChuckBuilds/ledmatrix-plugin-registry which doesn't exist. The
real registry is ChuckBuilds/ledmatrix-plugins. Fixed.
- "Next Steps" / "Questions to Resolve" sections were
pre-implementation planning notes. Replaced with a "Known
Limitations" section that documents the actually-real gaps
(sandboxing, resource limits, ratings, auto-updates).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/PLUGIN_STORE_GUIDE.md (1)
59-63:⚠️ Potential issue | 🟡 MinorUpdate UI navigation label to current tab model.
This still says “Plugin Store” tab, but current docs elsewhere describe install flow under Plugin Manager (with Plugin Store as a section), so this step is likely stale.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/PLUGIN_STORE_GUIDE.md` around lines 59 - 63, Update the UI navigation text in the step that currently reads "Navigate to the \"Plugin Store\" tab" to match the current UI model by changing it to "Navigate to the \"Plugin Manager\" tab" and, if needed for clarity, append a parenthetical or secondary instruction like "(then select the Plugin Store section)" so the doc's install flow (steps in PLUGIN_STORE_GUIDE.md) aligns with other docs referring to Plugin Manager.docs/PLUGIN_QUICK_REFERENCE.md (1)
154-167:⚠️ Potential issue | 🟠 MajorAlign registry example with the monorepo publishing model.
This section says official plugins live in
ChuckBuilds/ledmatrix-plugins, but the sample entry points to an individual plugin repo URL, which conflicts with that model and can mislead maintainers when preparing registry metadata.
Based on learnings: Official plugins must be published in theledmatrix-pluginsmonorepo, not as individual repositories.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/PLUGIN_QUICK_REFERENCE.md` around lines 154 - 167, The example registry entry in the plugins.json snippet incorrectly points the "repo" field at an individual plugin repository; update the sample to reference the monorepo publishing model used by the project by replacing the "repo" value for the "clock-simple" example with the official monorepo URL (e.g., the ledmatrix-plugins repo) and, if needed, indicate the package path or monorepo subfolder metadata instead of an external repo URL; ensure the example still shows the "id", "name", "author", "category", and "versions" fields but use the monorepo as the authoritative "repo" reference so maintainers are not misled.
🧹 Nitpick comments (1)
docs/REST_API_REFERENCE.md (1)
34-37: Avoid hardcoded source line numbers and route counts in docs.
app.py:607-615and “about 92 routes” will drift quickly; linking to files/sections without exact line numbers will age better.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/REST_API_REFERENCE.md` around lines 34 - 37, Remove hardcoded line numbers and the approximate route count from the docs: instead of "app.py:607-615" and "about 92 routes", refer to the source files and symbols (e.g., "SSE stream endpoints in web_interface/app.py" and "see the api_v3 blueprint in web_interface/blueprints/api_v3.py for the canonical list of routes") and add a note that route counts may change; this ensures the doc points to the files/symbols without brittle line numbers or exact counts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/ADVANCED_FEATURES.md`:
- Around line 796-803: The paragraph claims "The background data service is now
used by all of the sports scoreboard plugins" but the MLB entry still shows a
pending marker (⏳); update the MLB list item to reflect reality by either
removing the pending emoji and marking MLB as using the background service or
changing the paragraph to indicate that MLB is still pending; specifically edit
the section referencing each plugin's `background_service` block and the MLB
list item (currently showing "⏳ MLB (baseball)") so the text and the
`background_service` status for MLB are consistent.
- Around line 519-523: The text incorrectly states that REST/UI write to
`display_on_demand_config`; update the wording to say the REST endpoints and UI
write the on-demand request into the cache manager under the request key/path
(not `display_on_demand_config`), and clarify that `display_on_demand_config` is
instead used by the controller on activation—replace the key name in the
sentence and mention the request key/path used for ingestion.
---
Outside diff comments:
In `@docs/PLUGIN_QUICK_REFERENCE.md`:
- Around line 154-167: The example registry entry in the plugins.json snippet
incorrectly points the "repo" field at an individual plugin repository; update
the sample to reference the monorepo publishing model used by the project by
replacing the "repo" value for the "clock-simple" example with the official
monorepo URL (e.g., the ledmatrix-plugins repo) and, if needed, indicate the
package path or monorepo subfolder metadata instead of an external repo URL;
ensure the example still shows the "id", "name", "author", "category", and
"versions" fields but use the monorepo as the authoritative "repo" reference so
maintainers are not misled.
In `@docs/PLUGIN_STORE_GUIDE.md`:
- Around line 59-63: Update the UI navigation text in the step that currently
reads "Navigate to the \"Plugin Store\" tab" to match the current UI model by
changing it to "Navigate to the \"Plugin Manager\" tab" and, if needed for
clarity, append a parenthetical or secondary instruction like "(then select the
Plugin Store section)" so the doc's install flow (steps in
PLUGIN_STORE_GUIDE.md) aligns with other docs referring to Plugin Manager.
---
Nitpick comments:
In `@docs/REST_API_REFERENCE.md`:
- Around line 34-37: Remove hardcoded line numbers and the approximate route
count from the docs: instead of "app.py:607-615" and "about 92 routes", refer to
the source files and symbols (e.g., "SSE stream endpoints in
web_interface/app.py" and "see the api_v3 blueprint in
web_interface/blueprints/api_v3.py for the canonical list of routes") and add a
note that route counts may change; this ensures the doc points to the
files/symbols without brittle line numbers or exact counts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6a754d9d-3595-4fd9-8478-04d8f49f5dcc
📒 Files selected for processing (9)
docs/ADVANCED_FEATURES.mddocs/PLUGIN_CONFIGURATION_TABS.mddocs/PLUGIN_CONFIG_QUICK_START.mddocs/PLUGIN_DEPENDENCY_GUIDE.mddocs/PLUGIN_DEPENDENCY_TROUBLESHOOTING.mddocs/PLUGIN_QUICK_REFERENCE.mddocs/PLUGIN_REGISTRY_SETUP_GUIDE.mddocs/PLUGIN_STORE_GUIDE.mddocs/REST_API_REFERENCE.md
✅ Files skipped from review due to trivial changes (4)
- docs/PLUGIN_DEPENDENCY_GUIDE.md
- docs/PLUGIN_DEPENDENCY_TROUBLESHOOTING.md
- docs/PLUGIN_REGISTRY_SETUP_GUIDE.md
- docs/PLUGIN_CONFIGURATION_TABS.md
…ADMEs)
PLUGIN_ARCHITECTURE_SPEC.md
- Added a banner at the top noting this is a historical design doc
written before the plugin system shipped. The doc is ~1900 lines
with 13 stale /api/plugins/* paths (real is /api/v3/plugins/*),
references to web_interface_v2.py (current is app.py), and a
Migration Strategy / Implementation Roadmap that's now history.
Banner points readers at the current docs
(PLUGIN_DEVELOPMENT_GUIDE, PLUGIN_API_REFERENCE,
REST_API_REFERENCE) without needing to retrofit every section.
PLUGIN_CONFIG_ARCHITECTURE.md
- 10 occurrences of /api/plugins/* missing /v3 prefix. Bulk fixed.
DEVELOPER_QUICK_REFERENCE.md
- cache_manager.delete("key") -> cache_manager.clear_cache("key")
with comment noting delete() doesn't exist. Same bug already
documented in PLUGIN_API_REFERENCE.md.
SSH_UNAVAILABLE_AFTER_INSTALL.md
- 4 occurrences of port 5001 -> 5000 in AP-mode and Ethernet/WiFi
recovery instructions.
PLUGIN_CUSTOM_ICONS_FEATURE.md
- Port 5001 -> 5000.
CONFIG_DEBUGGING.md
- Documented /api/v3/config/plugin/<id> and /api/v3/config/validate
endpoints don't exist. Replaced with the real endpoints:
/api/v3/config/main, /api/v3/plugins/schema?plugin_id=,
/api/v3/plugins/config?plugin_id=. Added a note that validation
runs server-side automatically on POST.
STARLARK_APPS_GUIDE.md
- "Plugins -> Starlark Apps" UI navigation path doesn't exist (5
occurrences). Replaced with the real path: Plugin Manager tab,
then the per-plugin Starlark Apps tab in the second nav row.
- "Navigate to Plugins" install step -> Plugin Manager tab.
web_interface/README.md
- Documented several endpoints that don't exist in the api_v3
blueprint:
- GET /api/v3/plugins (list) -> /api/v3/plugins/installed
- GET /api/v3/plugins/<id> -> doesn't exist
- POST /api/v3/plugins/<id>/config -> POST /api/v3/plugins/config
- GET /api/v3/plugins/<id>/enable + /disable -> POST /api/v3/plugins/toggle
- GET /api/v3/store/plugins -> /api/v3/plugins/store/list
- POST /api/v3/store/install/<id> -> POST /api/v3/plugins/install
- POST /api/v3/store/uninstall/<id> -> POST /api/v3/plugins/uninstall
- POST /api/v3/store/update/<id> -> POST /api/v3/plugins/update
- POST /api/v3/display/start/stop/restart -> POST /api/v3/system/action
- GET /api/v3/display/status -> GET /api/v3/system/status
- Also fixed config/secrets.json -> config/config_secrets.json
- Replaced the per-section endpoint duplication with a current real
endpoint list and a pointer to docs/REST_API_REFERENCE.md.
- Documented that SSE stream endpoints are defined directly on the
Flask app at app.py:607-615, not in the api_v3 blueprint.
scripts/install/README.md
- Was missing 3 of the 9 install scripts in the directory:
one-shot-install.sh, configure_wifi_permissions.sh, and
debug_install.sh. Added them with brief descriptions.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
docs/PLUGIN_ARCHITECTURE_SPEC.md (3)
5-5: Optional: Consider simplifying "specific details" to "details".The phrase "specific details" could be simplified to just "details" for more concise writing. However, this is a minor style preference and the current phrasing is perfectly acceptable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/PLUGIN_ARCHITECTURE_SPEC.md` at line 5, Summary: Replace the phrase "specific details" with "details" for conciseness. Locate the occurrence of the phrase in PLUGIN_ARCHITECTURE_SPEC.md (the line containing "accurate, but specific details have drifted from the shipped") and edit it to read "accurate, but details have drifted from the shipped" (or similar grammatical variant), keeping surrounding punctuation and meaning intact.
31-31: Consider mentioning the fallback behavior.The PR objectives state "default plugins_directory is plugin-repos/ (with fallback to plugins/)", but this inline note only mentions
plugin-repos/as the actual default without mentioning the fallback. Consider adding the fallback detail for completeness and consistency with the banner at lines 12-13.📝 Suggested addition
-4. **Plugin Location**: `./plugins/` directory in project root *(actual default is now `plugin-repos/`)* +4. **Plugin Location**: `./plugins/` directory in project root *(actual default is now `plugin-repos/`, with fallback to `plugins/`)*🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/PLUGIN_ARCHITECTURE_SPEC.md` at line 31, Update the "Plugin Location" bullet in PLUGIN_ARCHITECTURE_SPEC.md to mention the fallback behavior: state that the actual default is "plugin-repos/" and that the system falls back to "plugins/" if "plugin-repos/" is not found, matching the banner note at the top; edit the line that currently reads "actual default is now `plugin-repos/`" to include "with fallback to `plugins/`" so the document is consistent.
11-11: Consider removing the specific line number reference.The reference to
web_interface/app.py:144includes a specific line number that may become stale as the codebase evolves. Consider either removing:144or replacing it with a more general reference like "blueprint registration inweb_interface/app.py" to reduce future maintenance burden.📝 Suggested revision
- blueprint is mounted at `/api/v3` (`web_interface/app.py:144`). + blueprint is mounted at `/api/v3` (see `web_interface/app.py`).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/PLUGIN_ARCHITECTURE_SPEC.md` at line 11, Update the sentence that currently reads "blueprint is mounted at `/api/v3` (`web_interface/app.py:144`)" to remove the hard-coded line number; change it to something like "blueprint is mounted at `/api/v3` (registered in web_interface/app.py)" or simply "blueprint is mounted at `/api/v3`" so the reference to web_interface/app.py is maintained without the brittle `:144` line number.docs/DEVELOPER_QUICK_REFERENCE.md (1)
59-74: Consider documenting both forms ofclear_cache().The quick reference shows
clear_cache("key")for clearing a specific key, but the method also supportsclear_cache()(with no argument) to clear all cache entries. Adding this variant would make the reference more complete.📝 Suggested enhancement
# Basic caching cached = cache_manager.get("key", max_age=3600) cache_manager.set("key", data) cache_manager.clear_cache("key") # there is no delete() method +cache_manager.clear_cache() # clear all cache entries🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/DEVELOPER_QUICK_REFERENCE.md` around lines 59 - 74, Update the Cache Manager Quick Methods docs to show both overloads of clear_cache: include an example for clearing a specific key using cache_manager.clear_cache("key") and add a second example showing the no-argument form cache_manager.clear_cache() to indicate it clears all entries; reference the cache_manager.get and cache_manager.set examples and ensure the doc text briefly notes the difference in effect between clear_cache("key") and clear_cache().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web_interface/README.md`:
- Around line 111-112: Update the README's reference to the SSE stream endpoints
to include the related explanatory comment; specifically, expand the cited range
that currently points to the three SSE route definitions to also cover the
nearby comment "Exempt SSE streams from CSRF and add rate limiting" so readers
see the routes and that contextual note together—edit the line-range reference
near the SSE stream description to include that comment line when pointing to
app.py.
- Around line 106-108: Add the missing `GET /api/v3/plugins/store/github-status`
endpoint to the Plugin Store section in README.md so the documentation matches
the implemented API; update the list under the "Plugin Store" heading (near the
existing `GET /api/v3/plugins/store/list` and `POST
/api/v3/plugins/store/refresh` entries) by inserting a new line documenting `GET
/api/v3/plugins/store/github-status - Get GitHub authentication status`.
- Around line 85-93: Update the Display & System Control documentation for the
POST /api/v3/system/action endpoint to include the two missing operations by
adding `enable_autostart` and `disable_autostart` to the action body list
alongside `start_display`, `stop_display`, `restart_display_service`,
`restart_web_service`, `git_pull`, `reboot_system`, and `shutdown_system`;
specifically modify the action body list under the `POST /api/v3/system/action`
entry so the list contains `enable_autostart` and `disable_autostart`.
---
Nitpick comments:
In `@docs/DEVELOPER_QUICK_REFERENCE.md`:
- Around line 59-74: Update the Cache Manager Quick Methods docs to show both
overloads of clear_cache: include an example for clearing a specific key using
cache_manager.clear_cache("key") and add a second example showing the
no-argument form cache_manager.clear_cache() to indicate it clears all entries;
reference the cache_manager.get and cache_manager.set examples and ensure the
doc text briefly notes the difference in effect between clear_cache("key") and
clear_cache().
In `@docs/PLUGIN_ARCHITECTURE_SPEC.md`:
- Line 5: Summary: Replace the phrase "specific details" with "details" for
conciseness. Locate the occurrence of the phrase in PLUGIN_ARCHITECTURE_SPEC.md
(the line containing "accurate, but specific details have drifted from the
shipped") and edit it to read "accurate, but details have drifted from the
shipped" (or similar grammatical variant), keeping surrounding punctuation and
meaning intact.
- Line 31: Update the "Plugin Location" bullet in PLUGIN_ARCHITECTURE_SPEC.md to
mention the fallback behavior: state that the actual default is "plugin-repos/"
and that the system falls back to "plugins/" if "plugin-repos/" is not found,
matching the banner note at the top; edit the line that currently reads "actual
default is now `plugin-repos/`" to include "with fallback to `plugins/`" so the
document is consistent.
- Line 11: Update the sentence that currently reads "blueprint is mounted at
`/api/v3` (`web_interface/app.py:144`)" to remove the hard-coded line number;
change it to something like "blueprint is mounted at `/api/v3` (registered in
web_interface/app.py)" or simply "blueprint is mounted at `/api/v3`" so the
reference to web_interface/app.py is maintained without the brittle `:144` line
number.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eb22f9fa-58a9-402b-b42b-ea2b890fdaf7
📒 Files selected for processing (9)
docs/CONFIG_DEBUGGING.mddocs/DEVELOPER_QUICK_REFERENCE.mddocs/PLUGIN_ARCHITECTURE_SPEC.mddocs/PLUGIN_CONFIG_ARCHITECTURE.mddocs/PLUGIN_CUSTOM_ICONS_FEATURE.mddocs/SSH_UNAVAILABLE_AFTER_INSTALL.mddocs/STARLARK_APPS_GUIDE.mdscripts/install/README.mdweb_interface/README.md
✅ Files skipped from review due to trivial changes (6)
- docs/PLUGIN_CUSTOM_ICONS_FEATURE.md
- docs/SSH_UNAVAILABLE_AFTER_INSTALL.md
- docs/PLUGIN_CONFIG_ARCHITECTURE.md
- docs/CONFIG_DEBUGGING.md
- docs/STARLARK_APPS_GUIDE.md
- scripts/install/README.md
PLUGIN_DEVELOPMENT_GUIDE.md
- Added a "Plugin directory note" callout near the top explaining
the plugins/ vs plugin-repos/ split:
- Dev workflow uses plugins/ (where dev_plugin_setup.sh creates
symlinks)
- Production / Plugin Store uses plugin-repos/ (the configurable
default per config.template.json:130)
- The plugin loader falls back to plugins/ so dev symlinks are
picked up automatically (schema_manager.py:77)
- User can set plugins_directory to "plugins" in the General tab
if they want both to share a directory
CLAUDE.md
- The Project Structure section had plugins/ and plugin-repos/
exactly reversed:
- Old: "plugins/ - Installed plugins directory (gitignored)"
"plugin-repos/ - Development symlinks to monorepo plugin dirs"
- Real: plugin-repos/ is the canonical Plugin Store install
location and is not gitignored. plugins/* IS gitignored
(verified in .gitignore) and is the legacy/dev location used by
scripts/dev/dev_plugin_setup.sh.
Reversed the descriptions and added line refs.
systemd/README.md
- "Manual Installation" section told users to copy the unit file
directly to /etc/systemd/system/. Verified the unit file in
systemd/ledmatrix.service contains __PROJECT_ROOT_DIR__
placeholders that the install scripts substitute at install time.
A user following the manual steps would get a service that fails
to start with "WorkingDirectory=__PROJECT_ROOT_DIR__" errors.
Added a clear warning and a sed snippet that substitutes the
placeholder before installing.
src/common/README.md
- Was missing 2 of the 11 utility modules in the directory
(verified with ls): permission_utils.py and cli.py. Added brief
descriptions for both.
Out-of-scope code bug found while auditing (flagged but not fixed):
- scripts/dev/dev_plugin_setup.sh:9 sets PROJECT_ROOT="$SCRIPT_DIR"
which resolves to scripts/dev/, not the project root. This means
the script's PLUGINS_DIR resolves to scripts/dev/plugins/ instead
of the project's plugins/ — confirmed by the existence of
scripts/dev/plugins/of-the-day/ from prior runs. Real fix is to
set PROJECT_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)". Not fixing in
this docs PR.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
These docs describe features that exist as documented in the doc but either never wired up or regressed when v3 shipped. Each gets a clear status banner so plugin authors don't waste time chasing features that don't actually work. FONT_MANAGER.md - The "For Plugin Developers / Plugin Font Registration" section documents adding a "fonts" block to manifest.json that gets registered via FontManager.register_plugin_fonts(). The method exists at src/font_manager.py:150 but is **never called from anywhere** in the codebase (verified: zero callers). A plugin shipping a manifest "fonts" block has its fonts silently ignored. Added a status warning and a note about how to actually ship plugin fonts (regular files in the plugin dir, loaded directly). PLUGIN_IMPLEMENTATION_SUMMARY.md - Added a top-level status banner. - Architecture diagram referenced src/plugin_system/registry_manager.py (which doesn't exist) and listed plugins/ as the install location. Replaced with the real file list (plugin_loader, schema_manager, health_monitor, operation_queue, state_manager) and pointed at plugin-repos/ as the default install location. - "Dependency Management: Virtual Environments" — verified there's no per-plugin venv. Removed the bullet and added a note that plugin Python deps install into the system Python environment, with no conflict resolution. - "Permission System: File Access Control / Network Access / Resource Limits / CPU and memory constraints" — none of these exist. There's a resource_monitor.py and health_monitor.py for metrics/warnings, but no hard caps or sandboxing. Replaced the section with what's actually implemented and a clear note that plugins run in the same process with full file/network access. PLUGIN_CUSTOM_ICONS.md and PLUGIN_CUSTOM_ICONS_FEATURE.md - The custom-icon feature was implemented in the v2 web interface via a getPluginIcon() helper in templates/index_v2.html that read the manifest "icon" field. When the v3 web interface was built, that helper wasn't ported. Verified in web_interface/templates/v3/base.html:515 and :774, plugin tab icons are hardcoded to `fas fa-puzzle-piece`. The "icon" field in plugin manifests is currently silently ignored (verified with grep across web_interface/ and src/plugin_system/ — zero non-action- related reads of plugin.icon or manifest.icon). - Added a status banner to both docs noting the regression so plugin authors don't think their custom icons are broken in their own plugin code. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The .cursor/ directory holds the dev-side helper docs that Cursor and
contributors using AI tooling rely on to bootstrap plugin development.
Several of them had the same bug patterns as the user-facing docs.
.cursor/plugin_templates/QUICK_START.md
- "Adding Image Rendering" section showed
display_manager.draw_image(image, x=0, y=0). That method doesn't
exist on DisplayManager (same bug as PLUGIN_API_REFERENCE.md and
PLUGIN_DEVELOPMENT_GUIDE.md). Replaced with the canonical
display_manager.image.paste((x,y)) pattern, including the
transparency-mask form.
.cursor/plugins_guide.md
- 10 occurrences of ./dev_plugin_setup.sh — the script lives at
scripts/dev/dev_plugin_setup.sh, so anyone copy-pasting these
examples gets "command not found". Bulk fixed via sed.
- "Test with emulator: python run.py --emulator" — there's no
--emulator flag. Replaced with the real options:
EMULATOR=true python3 run.py for the full display, or
scripts/dev_server.py for the dev preview.
- Secrets management section showed a fictional
"config_secrets": { "api_key": "my-plugin.api_key" } reference
field. Verified in src/config_manager.py:162-172 that secrets are
loaded by deep-merging config_secrets.json into the main config.
There is no separate reference field — just put the secret under
the same plugin namespace and read it from the merged config.
Rewrote the section with the real pattern.
- "ssh pi@raspberrypi" -> "ssh ledpi@your-pi-ip" (consistent with
the rest of LEDMatrix docs which use ledpi as the default user)
.cursor/README.md
- Same ./dev_plugin_setup.sh -> ./scripts/dev/dev_plugin_setup.sh
fix (×6 occurrences via replace_all).
- Same "python run.py --emulator" -> "EMULATOR=true python3 run.py"
fix. Also added a pointer to scripts/dev_server.py for previewing
plugins without running the full display.
- "Example Plugins: plugins/hockey-scoreboard/" — the canonical
source is the ledmatrix-plugins repo. Installed copies land in
plugin-repos/ or plugins/. Updated the line to point at the
ledmatrix-plugins repo and explain both local locations.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This is the file that Cursor reads to learn how plugin development works. Stale entries here directly mislead AI-assisted plugin authors on every new plugin. Several of the same bug patterns I've been fixing in the user-facing docs were here too. Display Manager section (highest impact) - "draw_image(image, x, y): Draw PIL Image" — that method doesn't exist on DisplayManager. Same bug already fixed in PLUGIN_API_REFERENCE.md, PLUGIN_DEVELOPMENT_GUIDE.md, ledmatrix-stocks/README.md, and .cursor/plugin_templates/QUICK_START.md. Removed the bullet and replaced it with a paragraph explaining the real pattern: paste onto display_manager.image directly, then update_display(). Includes the transparency-mask form. - Added the small_font/centered args to draw_text() since they're the ones that matter most for new plugin authors - Added draw_weather_icon since it's commonly used Cache Manager section - "delete(key): Remove cached value" — there's no delete() method on CacheManager. The real method is clear_cache(key=None) (also removes everything when called without args). Same bug as before. - Added get_cached_data_with_strategy and get_background_cached_data since contributors will hit these when working on sports plugins Plugin System Overview - "loaded from the plugins/ directory" — clarified that the default is plugin-repos/ (per config.template.json:130) with plugins/ as the dev fallback used by scripts/dev/dev_plugin_setup.sh Plugin Development Workflow - ./dev_plugin_setup.sh -> ./scripts/dev/dev_plugin_setup.sh (×2) - Manual setup step "Create directory in plugins/<plugin-id>/" -> plugin-repos/<plugin-id>/ as the canonical location - "Use emulator: python run.py --emulator or ./run_emulator.sh" — the --emulator flag doesn't exist; ./run_emulator.sh isn't at root (it lives at scripts/dev/run_emulator.sh). Replaced with the real options: scripts/dev_server.py for dev preview, or EMULATOR=true python3 run.py for the full emulator path. Configuration Management - "Reference secrets via config_secrets key in main config" — this is the same fictional reference syntax I just fixed in .cursor/plugins_guide.md. Verified in src/config_manager.py:162-172 that secrets are deep-merged into the main config; there's no separate reference field. Replaced with a clear explanation of the deep-merge approach. Code Organization - "plugins/<plugin-id>/" -> the canonical location is plugin-repos/<plugin-id>/ (or its dev-time symlink in plugins/) - "see plugins/hockey-scoreboard/ as reference" — the canonical source for example plugins is the ledmatrix-plugins repo. Updated the pointer. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/FONT_MANAGER.md (1)
155-204:⚠️ Potential issue | 🟡 MinorExample code is inconsistent with the warning.
The warning (lines 141-154) states that plugin font registration is not wired up and doesn't work. However, the example code in this section demonstrates using plugin fonts via
resolve_font()withfamily="custom_font", which assumes the font has been successfully registered.This example won't work until the registration is wired up, because "custom_font" won't exist in the font catalog. Consider either:
- Adding a note to this section that the example requires the planned registration to be completed
- Replacing this example with a working workaround (e.g., manual
register_plugin_fonts()call or PIL direct loading)- Removing the example until the feature is functional
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/FONT_MANAGER.md` around lines 155 - 204, The example in the Plugin Font Registration section shows using resolve_font (in PluginManager.display) as if plugin fonts from the manifest.json are already registered, but the warning states registration isn't wired up; update the docs to avoid confusion by either 1) adding a clear note above the example that this usage requires the planned plugin font registration to be implemented (i.e., the manifest entry won't work yet), or 2) replace the example with a working workaround that demonstrates explicitly registering/loading the font (e.g., call/register_plugin_fonts or a manual PIL font load prior to calling font_manager.resolve_font), and reference resolve_font, PluginManager.display, and the manifest.json font entries so readers can find the relevant code paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.cursor/plugins_guide.md:
- Around line 403-406: The sentence "There is no `--emulator` flag" is
incorrect; update the section so it acknowledges the `--emulator` flag exists
and show the correct usage. Replace or remove that sentence and instead
reference the `run.py` flag usage (e.g., `python run.py --emulator` or
`EMULATOR=true python3 run.py`) and ensure consistency with other mentions (see
`run.py` where `parser.add_argument('-e', '--emulator', ...)` is defined and
earlier examples like `python run.py --emulator`). Make this change so the doc
no longer contradicts lines that call `python run.py --emulator`.
In @.cursorrules:
- Around line 73-78: Update the documentation text to remove the incorrect
statement "There is no `--emulator` flag" and state that emulator mode can be
enabled either via the environment variable or the CLI flag; specifically
mention both "EMULATOR=true python3 run.py" and "python3 run.py --emulator" (or
"-e/--emulator") as valid options, referencing the flag defined in run.py via
parser.add_argument('-e', '--emulator', action='store_true') to make it clear
the CLI flag is supported.
- Around line 177-184: The code calls the non-existent cache_manager.delete()
which raises AttributeError; update both call sites to use the documented API:
replace cache_manager.delete(key) (or cache_manager.delete() when key is
omitted) with cache_manager.clear_cache(key) (or cache_manager.clear_cache() to
clear all) — specifically fix the call inside the clear_cache() function in the
API helper and the call in ResourceMonitor (resource_monitor.py) around the
background/monitoring flow; alternatively, if you prefer preserving a delete()
semantic, add a simple cache_manager.delete(key) wrapper that forwards to
clear_cache(key).
- Around line 5-11: Documentation incorrectly implies the plugin loader falls
back to "plugins/" for main discovery; clarify that
PluginManager.discover_plugins() only scans the configured
plugin_system.plugins_directory and does not perform a fallback, while the
fallback to "plugins/" exists only in the store operations implemented in
store_manager (the fallback logic there should be referenced). Update the
.cursorrules text to state that the loader's fallback is limited to
store_manager's store-related operations and that
PluginManager.discover_plugins() uses only the configured directory.
In `@docs/FONT_MANAGER.md`:
- Around line 152-153: Remove the incorrect claim that the "Fonts" tab works for
elements registered via register_manager_font(): update the docs to delete or
reword the sentence referencing the Fonts tab and its interaction with
register_manager_font(), and add a short note (if needed) clarifying that the
/fonts/overrides endpoints are placeholders returning empty overrides and there
is no functional integration between register_manager_font() and the
overrides/UI at this time.
- Around line 148-151: Doc text is misleading about loading plugin fonts via
FontManager; clarify how to actually register and reference plugin fonts or
state the workaround. Update the docs to explain that
FontManager.resolve_font(family=...) expects a family name (not a file path),
that plugin-relative URIs use the font manifest "source" (plugin://) and require
calling register_plugin_fonts(...) from the plugin init (e.g., from __init__),
and that set_override(...) has no "source" param so it cannot create plugin://
entries; alternatively state the supported workaround is to load font files
directly with PIL if register_plugin_fonts isn't wired. Mention the relevant
symbols resolve_font, register_plugin_fonts, set_override, and FontManager so
readers can find the APIs.
---
Outside diff comments:
In `@docs/FONT_MANAGER.md`:
- Around line 155-204: The example in the Plugin Font Registration section shows
using resolve_font (in PluginManager.display) as if plugin fonts from the
manifest.json are already registered, but the warning states registration isn't
wired up; update the docs to avoid confusion by either 1) adding a clear note
above the example that this usage requires the planned plugin font registration
to be implemented (i.e., the manifest entry won't work yet), or 2) replace the
example with a working workaround that demonstrates explicitly
registering/loading the font (e.g., call/register_plugin_fonts or a manual PIL
font load prior to calling font_manager.resolve_font), and reference
resolve_font, PluginManager.display, and the manifest.json font entries so
readers can find the relevant code paths.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f660879f-d01b-4cc5-be63-63d218ea160e
📒 Files selected for processing (8)
.cursor/README.md.cursor/plugin_templates/QUICK_START.md.cursor/plugins_guide.md.cursorrulesdocs/FONT_MANAGER.mddocs/PLUGIN_CUSTOM_ICONS.mddocs/PLUGIN_CUSTOM_ICONS_FEATURE.mddocs/PLUGIN_IMPLEMENTATION_SUMMARY.md
✅ Files skipped from review due to trivial changes (5)
- docs/PLUGIN_CUSTOM_ICONS.md
- .cursor/plugin_templates/QUICK_START.md
- .cursor/README.md
- docs/PLUGIN_CUSTOM_ICONS_FEATURE.md
- docs/PLUGIN_IMPLEMENTATION_SUMMARY.md
LICENSE
- The repository previously had no LICENSE file. The README and every
downstream plugin README already reference GPL-3.0 ("same as
LEDMatrix project"), but the canonical license text was missing —
contributors had no formal record of what they were contributing
under, and GitHub couldn't auto-detect the license for the repo
banner.
- Added the canonical GPL-3.0 text from
https://www.gnu.org/licenses/gpl-3.0.txt (verbatim, 674 lines).
- Compatibility verified: rpi-rgb-led-matrix is GPL-2.0-or-later
(per its COPYING file and README; the "or any later version" clause
in lib/*.h headers makes GPL-3.0 distribution legal).
CONTRIBUTING.md
- The repository had no CONTRIBUTING file. New contributors had to
reconstruct the dev setup from DEVELOPMENT.md, PLUGIN_DEVELOPMENT_GUIDE.md,
SUBMISSION.md, and the root README.
- Added a single page covering: dev environment setup (preview
server, emulator, hardware), running tests, PR submission flow,
commit message convention, plugin contribution pointer, and the
license terms contributors are agreeing to.
> Note for the maintainer: I (the AI assistant doing this audit) am
> selecting GPL-3.0 because every reference in the existing
> documentation already says GPL-3.0 — this commit just makes that
> declaration legally binding by adding the actual file. Please
> confirm during PR review that GPL-3.0 is what you want; if you
> prefer a different license, revert this commit before merging.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Tier 1 organizational files that any open-source project at LEDMatrix's maturity is expected to have. None of these existed before. They're additive — no existing content was rewritten. CODE_OF_CONDUCT.md - Contributor Covenant 2.1 (the de facto standard for open-source projects). Mentions both the Discord and the GitHub Security Advisories channel for reporting violations. SECURITY.md - Private vulnerability disclosure flow with two channels: GitHub Security Advisories (preferred) and Discord DM. - Documents the project's known security model as intentional rather than vulnerabilities: no web UI auth, plugins run unsandboxed, display service runs as root for GPIO access, config_secrets.json is plaintext. These match the limitations already called out in PLUGIN_QUICK_REFERENCE.md and the audit flagging from earlier in this PR. - Out-of-scope section points users at upstream (rpi-rgb-led-matrix, third-party plugins) so reports land in the right place. .github/PULL_REQUEST_TEMPLATE.md - 10-line checklist that prompts for the things that would have caught the bugs in this very PR: did you load the changed plugin once, did you update docs alongside code, are there any plugin compatibility implications. - Linked from CONTRIBUTING.md for the full flow. README.md - Added a License section near the bottom (the README previously said nothing about the license despite the project being GPL-3.0). - Added a Contributing section pointing at CONTRIBUTING.md and SECURITY.md. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The bug_report.md template was the GitHub default and asked "Desktop (OS/Browser/Version)" and "Smartphone (Device/OS)" — neither of which is relevant for a project that runs on a Raspberry Pi with hardware LED panels. A user filing a bug under the old template was giving us none of the information we'd actually need to triage it. Replaced with a LEDMatrix-aware template that prompts for: - Pi model, OS/kernel, panel type, HAT/Bonnet, PWM jumper status, display chain dimensions - LEDMatrix git commit / release tag - Plugin id and version (if the bug is plugin-related) - Relevant config snippet (with redaction reminder for API keys) - journalctl log excerpt with the exact command to capture it - Optional photo of the actual display for visual issues Kept feature_request.md as-is — generic content there is fine. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Found 5 more bare /api/plugins/* paths in PLUGIN_CONFIGURATION_TABS.md
that I missed in the round 2 sweep — they're inside data flow diagrams
and prose ("loaded via /api/plugins/installed", etc.) so the earlier
grep over Markdown code blocks didn't catch them. Fixed all 5 to use
/api/v3/plugins/* (the api_v3 blueprint mount path verified at
web_interface/app.py:144).
Also added a status banner noting that the "Implementation Details"
section references the pre-v3 file layout (web_interface_v2.py,
templates/index_v2.html) which no longer exists. The current
implementation is in web_interface/app.py, blueprints/api_v3.py, and
templates/v3/. Same kind of historical drift I flagged in
PLUGIN_ARCHITECTURE_SPEC.md and the PLUGIN_CUSTOM_ICONS_FEATURE doc.
The user-facing parts of the doc (Overview, Features, Form Generation
Process) are still accurate.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The widget registry README documented 3 widgets (file-upload,
checkbox-group, custom-feeds) but the directory contains 23 registered
widgets total. A plugin author reading this doc would think those 3
were the only built-in options and either reach for a custom widget
unnecessarily or settle for a generic text input.
Verified the actual list with:
grep -h "register('" web_interface/static/v3/js/widgets/*.js \
| sed -E "s|.*register\\('([^']+)'.*|\\1|" | sort -u
Added an "Other Built-in Widgets" section after the 3 detailed
sections, listing the remaining 20 with one-line descriptions
organized by category:
- Inputs (6): text-input, textarea, number-input, email-input,
url-input, password-input
- Selectors (7): select-dropdown, radio-group, toggle-switch,
slider, color-picker, font-selector, timezone-selector
- Date/time/scheduling (4): date-picker, day-selector, time-range,
schedule-picker
- Composite/data-source (2): array-table, google-calendar-picker
- Internal (2): notification, base-widget
Pointed at the .js source files as the canonical source for each
widget's exact schema and options — keeps this list low-maintenance
since I'm not duplicating each widget's full options table.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
scripts/README_NBA_LOGOS.md
- "python download_nba_logos.py" — wrong on two counts. The script
is at scripts/download_nba_logos.py (not the project root), and
"python" is Python 2 on most systems. Replaced all 4 occurrences
with "python3 scripts/download_nba_logos.py".
- The doc framed itself as the way to set up "the NBA leaderboard".
The basketball/leaderboard functionality is now in the
basketball-scoreboard and ledmatrix-leaderboard plugins (in the
ledmatrix-plugins repo), which auto-download logos on first run.
Reframed the script as a pre-population utility for offline / dev
use cases.
- Bumped the documented Python minimum from 3.7 to 3.9 to match
the rest of the project.
docs/PLUGIN_CONFIGURATION_GUIDE.md
- The "Plugin Manifest" example was missing 3 fields the plugin
loader actually requires: id, entry_point, and class_name. A
contributor copying this manifest verbatim would get
PluginError("No class_name in manifest") at load time — the same
loader bug already found in stock-news. Added all three.
- The same example showed config_schema as an inline object. The
loader expects config_schema to be a file path string (e.g.
"config_schema.json") with the actual schema in a separate JSON
file — verified earlier in this audit. Fixed.
- Added a paragraph explaining the loader's required fields and
the case-sensitivity rule on class_name (the bug that broke
hello-world's manifest before this PR fixed it).
- "Plugin Manager Class" example had the wrong constructor
signature: (config, display_manager, cache_manager, font_manager).
The real BasePlugin.__init__ at base_plugin.py:53-60 takes
(plugin_id, config, display_manager, cache_manager, plugin_manager).
A copy-pasted example would TypeError on instantiation. Fixed,
including a comment noting which attributes BasePlugin sets up.
- Renamed the example class from MyPluginManager to MyPlugin to
match the project convention (XxxPlugin / XxxScoreboardPlugin
in actual plugins).
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ask-Limiter)
A doc-vs-code crosscheck of every Python import in src/ and
web_interface/ against requirements.txt found 3 packages that the
code uses but requirements.txt doesn't list. Verified with grep that
all 3 are wrapped in try/except blocks with documented fallback
paths, so they're optional features rather than missing required
deps:
- scipy src/common/scroll_helper.py:26
→ from scipy.ndimage import shift; HAS_SCIPY flag.
Used for sub-pixel interpolation in scrolling.
Falls back to a simpler shift algorithm without it.
- psutil src/plugin_system/resource_monitor.py:15
→ import psutil; PSUTIL_AVAILABLE flag. Used for
per-plugin CPU/memory monitoring. Silently no-ops
without it.
- flask-limiter web_interface/app.py:42-43
→ from flask_limiter import Limiter; wrapped at the
caller. Used for accidental-abuse rate limiting on
the web interface (not security). Web interface
starts without rate limiting when missing.
These were latent in two ways:
1. A user reading requirements.txt thinks they have the full feature
set after `pip install -r requirements.txt`, but they don't get
smoother scrolling, plugin resource monitoring, or rate limiting.
2. A contributor who deletes one of the packages from their dev env
wouldn't know which feature they just lost — the fallbacks are
silent.
Added an "Optional dependencies" section at the bottom of
requirements.txt with the version constraint, the file:line where
each is used, the feature it enables, and the install command. The
comment-only format means `pip install -r requirements.txt` still
gives the minimal-feature install (preserving current behavior),
while users who want the full feature set can copy the explicit
pip install commands.
Other findings from the same scan that came back as false positives
or known issues:
- web_interface_v2: dead pattern flagged in earlier iteration
(still no real implementation; affects 11+ plugins via the same
try/except dead-fallback pattern)
- urllib3: comes with `requests` transitively
- All 'src.', 'web_interface.', 'rgbmatrix', 'RGBMatrixEmulator'
imports: internal modules
- base_plugin / plugin_manager / store_manager / mocks /
visual_display_manager: relative imports to local modules
- freetype: false positive (freetype-py is in requirements.txt
under the package name)
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Ran a doc-vs-filesystem crosscheck: extracted every backtick-quoted path with a file extension or known directory prefix from docs/*.md and verified each exists. After filtering false positives (placeholder paths, config keys mistaken for paths, paths inside docs that already have historical-status banners), found 4 real broken references — 3 fixed in docs, 1 fixed by creating the missing file: docs/HOW_TO_RUN_TESTS.md:339 - Claimed ".github/workflows/tests.yml" exists and runs pytest on multiple Python versions in CI. There is no such workflow. The only GitHub Actions file is security-audit.yml (bandit + semgrep). - Pytest runs locally but is NOT gated on PRs. - Replaced the fictional CI section with the actual state and a note explaining how someone could contribute a real test workflow. docs/MIGRATION_GUIDE.md:92 - Referenced scripts/fix_perms/README.md "(if exists)" — the hedge betrays that the writer wasn't sure. The README didn't exist. The 6 scripts in scripts/fix_perms/ were never documented. - Created the missing scripts/fix_perms/README.md from scratch with one-line descriptions of all 6 scripts (fix_assets, fix_cache, fix_plugin, fix_web, fix_nhl_cache, safe_plugin_rm) + when-to-use-each guidance + usage examples. - Updated MIGRATION_GUIDE link to drop the "(if exists)" hedge since the file now exists. docs/FONT_MANAGER.md:376 - "See test/font_manager_example.py for a complete working example" — that file does not exist. Verified by listing test/ directory. - Replaced with a pointer to src/font_manager.py itself and the existing scoreboard base classes in src/base_classes/ that actually use the font manager API in production. Path-existence check methodology: - Walked docs/ recursively, regex-extracted backtick-quoted paths matching either /\.(py|sh|json|yml|yaml|md|txt|service|html|js|css|ttf|bdf|png)/ or paths starting with known directory prefixes (scripts/, src/, config/, web_interface/, systemd/, assets/, docs/, test/, etc.) - Filtered out URLs, absolute paths (placeholders), and paths without slashes (likely not relative refs). - Checked existence relative to project root. - Out of 80 unique relative paths in docs/, 32 didn't exist on disk. Most were false positives (configkeys mistaken for paths, example placeholders like 'assets/myfont.ttf', historical references inside docs that already have status banners). The 4 above were genuine broken refs. This pattern is reusable for future iterations and worth wiring into CI (link checker like lychee, scoped to fenced code paths rather than just markdown links, would catch the same class). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reviewed all 12 CodeRabbit comments on PR #306, verified each against the current code, and fixed the 11 valid ones. The 12th finding is a real code bug (cache_manager.delete() calls in api_helper.py and resource_monitor.py) that's already in the planned follow-up code-fix PR, so it stays out of this docs PR. Fixed: .cursor/plugins_guide.md, .cursor/README.md, .cursorrules - I claimed "there is no --emulator flag" in 3 places. Verified in run.py:19-20 that the -e/--emulator flag is defined and functional (it sets os.environ["EMULATOR"]="true" before the display imports). Other docs I didn't touch (.cursor/plugin_templates/QUICK_START.md, docs/PLUGIN_DEVELOPMENT_GUIDE.md) already use the flag correctly. Replaced all 3 wrong statements with accurate guidance that both forms work and explains the CLI flag's relationship to the env var. .cursorrules, docs/GETTING_STARTED.md, docs/WEB_INTERFACE_GUIDE.md, docs/PLUGIN_DEVELOPMENT_GUIDE.md - Four places claimed "the plugin loader also falls back to plugins/". Verified that PluginManager.discover_plugins() (src/plugin_system/plugin_manager.py:154) only scans the configured directory — no fallback. The fallback to plugins/ exists only in two narrower places: store_manager.py:1700-1718 (store install/update/uninstall operations) and schema_manager.py:70-80 (schema lookup for the web UI form generator). Rewrote all four mentions with the precise scope. Added a recommendation to set plugin_system.plugins_directory to "plugins" for the smoothest dev workflow with dev_plugin_setup.sh symlinks. docs/FONT_MANAGER.md - The "Status" warning told plugin authors to use display_manager.font_manager.resolve_font(...) as a workaround for loading plugin fonts. Verified in src/font_manager.py that resolve_font() takes a family name, not a file path — so the workaround as written doesn't actually work. Rewrote to tell authors to load the font directly with PIL or freetype-py in their plugin. - The same section said "the user-facing font override system in the Fonts tab still works for any element that's been registered via register_manager_font()". Verified in web_interface/blueprints/api_v3.py:5404-5428 that /api/v3/fonts/overrides is a placeholder implementation that returns empty arrays and contains "would integrate with the actual font system" comments — the Fonts tab does not have functional integration with register_manager_font() or the override system. Removed the false claim and added an explicit note that the tab is a placeholder. docs/ADVANCED_FEATURES.md:523 - The on-demand section said REST/UI calls write a request "into the cache manager (display_on_demand_config key)". Wrong — verified via grep that api_v3.py:1622 and :1687 write to display_on_demand_request, and display_on_demand_config is only written by the controller during activation (display_controller.py:1195, cleared at :1221). Corrected the key name and added controller file:line references so future readers can verify. docs/ADVANCED_FEATURES.md:803 - "Plugins using the background service" paragraph listed all scoreboard plugins but an orphaned "⏳ MLB (baseball)" bullet remained below from the old version of the section. Removed the orphan and added "baseball/MLB" to the inline list for clarity. web_interface/README.md - The POST /api/v3/system/action action list was incomplete. Verified in web_interface/app.py:1383,1386 that enable_autostart and disable_autostart are valid actions. Added both. - The Plugin Store section was missing GET /api/v3/plugins/store/github-status (verified at api_v3.py:3296). Added it. - The SSE line-range reference was app.py:607-615 but line 619 contains the "Exempt SSE streams from CSRF and add rate limiting" block that's semantically part of the same feature. Extended the range to 607-619. docs/GETTING_STARTED.md - Rows/Columns step said "Columns: 64 or 96 (match your hardware)". The web UI's validation accepts any integer in 16-128. Clarified that 64 and 96 are the common bundled-hardware values but the valid range is wider. Not addressed (out of scope for docs PR): - .cursorrules:184 CodeRabbit comment flagged the non-existent cache_manager.delete() calls in src/common/api_helper.py:287 and src/plugin_system/resource_monitor.py:343. These are real CODE bugs, not doc bugs, and they're the first item in the planned post-docs-refresh code-cleanup PR (see /home/chuck/.claude/plans/warm-imagining-river.md). The docs in this PR correctly state that delete() doesn't exist on CacheManager — the fix belongs in the follow-up code PR that either adds a delete() shim or updates the two callers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ChuckBuilds
pushed a commit
that referenced
this pull request
Apr 8, 2026
…, CI) The docs refresh effort (#306, ledmatrix-plugins#92) surfaced seven code bugs that were intentionally left out of the docs PRs because they required code changes rather than doc fixes. This PR addresses the six that belong in LEDMatrix (the seventh — a lacrosse-scoreboard mode rename — lives in the plugins repo). Bug 1: cache_manager.delete() AttributeError src/common/api_helper.py:287 and src/plugin_system/resource_monitor.py:343 both call cache_manager.delete(key), which doesn't exist — only clear_cache(key=None). Added a delete() alias method on CacheManager that forwards to clear_cache(key). Reverts the "There is no delete() method" wording in DEVELOPER_QUICK_REFERENCE, .cursorrules so the docs match the new shim. Bug 2: dev_plugin_setup.sh PROJECT_ROOT resolution scripts/dev/dev_plugin_setup.sh:9 set PROJECT_ROOT to SCRIPT_DIR instead of walking up two levels to the repo root, so PLUGINS_DIR resolved to scripts/dev/plugins/ and created symlinks under the script's own directory. Fixed the path and removed the stray scripts/dev/plugins/of-the-day symlink left by earlier runs. Bug 3: plugin custom icons regressed from v2 to v3 web_interface/blueprints/api_v3.py built the /plugins/installed response without including the manifest's "icon" field, and web_interface/templates/v3/base.html hardcoded fas fa-puzzle-piece in all three plugin-tab render sites. Pass the icon through the API and read it from the templates with a puzzle-piece fallback. Reverts the "currently broken" banners in docs/PLUGIN_CUSTOM_ICONS.md and docs/PLUGIN_CUSTOM_ICONS_FEATURE.md. Bug 4: register_plugin_fonts was never wired up src/font_manager.py:150 defines register_plugin_fonts(plugin_id, font_manifest) but nothing called it, so plugin manifests with a "fonts" block were silently no-ops. Wired the call into PluginManager.load_plugin() right after plugin_loader.load_plugin returns. Reverts the "not currently wired" warning in docs/FONT_MANAGER.md's "For Plugin Developers" section. Bug 5: dead web_interface_v2 import pattern (LEDMatrix half) src/base_odds_manager.py had a try/except importing web_interface_v2.increment_api_counter, falling back to a no-op stub. The module doesn't exist anywhere in the v3 codebase and no API metrics dashboard reads it. Deleted the import block and the single call site; the plugins-repo half of this cleanup lands in ledmatrix-plugins#<next>. Bug 7: no CI test workflow .github/workflows/ only contained security-audit.yml; pytest ran locally but was not gated on PRs. Added .github/workflows/tests.yml running pytest against Python 3.10, 3.11, 3.12 in EMULATOR=true mode, skipping tests marked hardware or slow. Updated docs/HOW_TO_RUN_TESTS.md to reflect that the workflow now exists. Verification done locally: - CacheManager.delete(key) round-trips with set/get - base_odds_manager imports without the v2 module present - dev_plugin_setup.sh PROJECT_ROOT resolves to repo root - api_v3 and plugin_manager compile clean - tests.yml YAML parses - Script syntax check on dev_plugin_setup.sh Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
4 tasks
ChuckBuilds
added a commit
that referenced
this pull request
Apr 8, 2026
#307) * fix: post-audit follow-up code fixes (cache, fonts, icons, dev script, CI) The docs refresh effort (#306, ledmatrix-plugins#92) surfaced seven code bugs that were intentionally left out of the docs PRs because they required code changes rather than doc fixes. This PR addresses the six that belong in LEDMatrix (the seventh — a lacrosse-scoreboard mode rename — lives in the plugins repo). Bug 1: cache_manager.delete() AttributeError src/common/api_helper.py:287 and src/plugin_system/resource_monitor.py:343 both call cache_manager.delete(key), which doesn't exist — only clear_cache(key=None). Added a delete() alias method on CacheManager that forwards to clear_cache(key). Reverts the "There is no delete() method" wording in DEVELOPER_QUICK_REFERENCE, .cursorrules so the docs match the new shim. Bug 2: dev_plugin_setup.sh PROJECT_ROOT resolution scripts/dev/dev_plugin_setup.sh:9 set PROJECT_ROOT to SCRIPT_DIR instead of walking up two levels to the repo root, so PLUGINS_DIR resolved to scripts/dev/plugins/ and created symlinks under the script's own directory. Fixed the path and removed the stray scripts/dev/plugins/of-the-day symlink left by earlier runs. Bug 3: plugin custom icons regressed from v2 to v3 web_interface/blueprints/api_v3.py built the /plugins/installed response without including the manifest's "icon" field, and web_interface/templates/v3/base.html hardcoded fas fa-puzzle-piece in all three plugin-tab render sites. Pass the icon through the API and read it from the templates with a puzzle-piece fallback. Reverts the "currently broken" banners in docs/PLUGIN_CUSTOM_ICONS.md and docs/PLUGIN_CUSTOM_ICONS_FEATURE.md. Bug 4: register_plugin_fonts was never wired up src/font_manager.py:150 defines register_plugin_fonts(plugin_id, font_manifest) but nothing called it, so plugin manifests with a "fonts" block were silently no-ops. Wired the call into PluginManager.load_plugin() right after plugin_loader.load_plugin returns. Reverts the "not currently wired" warning in docs/FONT_MANAGER.md's "For Plugin Developers" section. Bug 5: dead web_interface_v2 import pattern (LEDMatrix half) src/base_odds_manager.py had a try/except importing web_interface_v2.increment_api_counter, falling back to a no-op stub. The module doesn't exist anywhere in the v3 codebase and no API metrics dashboard reads it. Deleted the import block and the single call site; the plugins-repo half of this cleanup lands in ledmatrix-plugins#<next>. Bug 7: no CI test workflow .github/workflows/ only contained security-audit.yml; pytest ran locally but was not gated on PRs. Added .github/workflows/tests.yml running pytest against Python 3.10, 3.11, 3.12 in EMULATOR=true mode, skipping tests marked hardware or slow. Updated docs/HOW_TO_RUN_TESTS.md to reflect that the workflow now exists. Verification done locally: - CacheManager.delete(key) round-trips with set/get - base_odds_manager imports without the v2 module present - dev_plugin_setup.sh PROJECT_ROOT resolves to repo root - api_v3 and plugin_manager compile clean - tests.yml YAML parses - Script syntax check on dev_plugin_setup.sh Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address CodeRabbit review comments on #307 - src/cache_manager.py: clear_cache(key) treated empty string as "wipe all" because of `if key:`. Switched to `key is None` branching, made delete(key) and clear_cache(key) reject empty strings and None outright with ValueError, and updated both docstrings to make the contract explicit. Verified locally with a round-trip test that clear_cache() (no arg) still wipes everything but clear_cache("") and delete("") raise. - src/plugin_system/plugin_manager.py: was reaching for the font manager via getattr(self.display_manager, 'font_manager', None). PluginManager already takes a dedicated font_manager parameter (line 54) and stores it as self.font_manager (line 69), so the old path was both wrong and could miss the font manager entirely when the host injects them separately. Switched to self.font_manager directly with the same try/except warning behavior. - web_interface/templates/v3/base.html: in the full plugin-tab renderer, the icon was injected with `<i class="${escapeHtml(plugin.icon)}">` — but escapeHtml only escapes <, >, and &, not double quotes, so a manifest with a quote in its icon string could break out of the class attribute. Replaced the innerHTML template with createElement for the <i> tag, set className from plugin.icon directly (no string interpolation), and used a text node for the label. Same fix shape would also harden the two stub-renderer sites at line 515 / 774, but those already escape `"` to " and CodeRabbit only flagged this site, so leaving them for now. - docs/FONT_MANAGER.md: clarified that the Manual Font Overrides *workflow* (set_override / remove_override / font_overrides.json) is the supported override path today, and only the Fonts tab in the web UI is the placeholder. Previous wording conflated the two and made it sound like overrides themselves were broken. - docs/HOW_TO_RUN_TESTS.md: replaced the vague "see the PR adding it" with a concrete link to #307 and a note that the workflow file itself is held back pending the workflow scope. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Chuck <chuck@example.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Walked the README and
docs/tree against current code and fixed several real bugs and many stale references. This is the LEDMatrix half of a two-PR docs refresh; the matching plugins-repo PR fixes the per-plugin READMEs.User-facing fixes
docs/GETTING_STARTED.mdanddocs/TROUBLESHOOTING.md(17 occurrences) said 5050, butweb_interface/start.py:123actually binds 5000. A user following the old docs would never reach the web UI.GETTING_STARTED.mdwalked users through "Plugin Store", "Plugin Management", "Sports Configuration", "Durations", and "Font Management" tabs — none of which exist. Verified the real tab list againstweb_interface/templates/v3/base.html:935-1000and rewrote the steps. Real tabs: Overview, General, WiFi, Schedule, Display, Config Editor, Fonts, Logs, Cache, Operation History, Plugin Manager (+ per-plugin tabs).partials/plugin_config.html:792).TROUBLESHOOTING.mdtold users to run./troubleshoot_weather.shwhich doesn't exist anywhere in the repo. Replaced with a note that weather is now anledmatrix-pluginsplugin.README.mdsaidchmod +x install_web_service.sh/sudo ./install_web_service.sh, but the file lives atscripts/install/install_web_service.sh.Developer-facing fixes
draw_image()doesn't exist:PLUGIN_API_REFERENCE.mdandPLUGIN_DEVELOPMENT_GUIDE.mdboth documented adisplay_manager.draw_image(image, x, y)method that has never existed onDisplayManager. Real plugins paste ontodisplay_manager.imagedirectly (verified insrc/base_classes/{baseball,basketball,football,hockey}.py). Fixed both docs with the canonical pattern, including the transparency-mask form.cache_manager.delete()doesn't exist:PLUGIN_API_REFERENCE.mddocumented it. Real method isclear_cache(key=None). Note: while fixing this I discovered that production code insrc/plugin_system/resource_monitor.py:343andsrc/common/api_helper.py:287also callscache_manager.delete()and wouldAttributeErrorat runtime. Out of scope for this docs PR but flagging it — please decide whether to add adelete()shim or convert the call sites.BasePluginmethods: the API reference's "Optional Methods" section listed 7 methods, butBasePluginexposes 17. Added documented sections for the dynamic-duration hooks (supports_dynamic_duration,get_dynamic_duration_cap,is_cycle_complete,reset_cycle_state), live priority (has_live_priority), and the Vegas-mode interface (get_vegas_content,get_vegas_content_type,get_vegas_display_mode,get_supported_vegas_modes,get_vegas_segment_width).DEVELOPMENT.mdhad a whole section on initializing plugins as git submodules of this repo. They're not —.gitmodulesonly containsrpi-rgb-led-matrix-master. Plugins are installed at runtime into the directory configured byplugin_system.plugins_directory(defaultplugin-repos/). Both internal links in this doc were also broken (relative path off by one level).plugins_directoryisplugin-repos/(verified inconfig/config.template.json:130,src/display_controller.py:132,web_interface/templates/v3/partials/general.html:162), notplugins/. The loader does fall back toplugins/(src/plugin_system/schema_manager.py:77).docs/HOW_TO_RUN_TESTS.md: removedpytest-timeoutfrom the install line (not inrequirements.txt); replaced the bogustest/integration/reference (the real integration tests live attest/web_interface/integration/); rewrote the test file structure diagram to match the actual layout.docs/EMULATOR_SETUP_GUIDE.md: clone URL was a placeholder; the documented defaultpixel_sizewas 16 but the realemulator_config.jsonships with 5.Index
docs/README.md: rewrote. The old index claimed "16-17 files after consolidation" butdocs/actually has 38 .md files. Four were missing from the index entirely:CONFIG_DEBUGGING.md,DEV_PREVIEW.md,PLUGIN_ERROR_HANDLING.md,STARLARK_APPS_GUIDE.md. Added them. Trimmed the navel-gazing "Recent Consolidations" / "Statistics" sections.Files changed
Test plan
docs/GETTING_STARTED.mdwalks correctly against the real web UIDEVELOPMENT.md(plugin-repos/) match what's actually expectedcache_manager.delete()issue flagged above and decide on a follow-upsrc/plugin_system/base_plugin.py🤖 Generated with Claude Code
Summary by CodeRabbit