docs: address CodeRabbit review comments on #306

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>

.cursorrules

@@ -195,9 +195,8 @@ Located in: `src/cache_manager.py`
 **Key Methods:**
 - `get(key, max_age=300)`: Get cached value (returns None if missing/stale)
 - `set(key, value, ttl=None)`: Cache a value
-- `delete(key)` / `clear_cache(key=None)`: Remove a single cache entry,
-  or (for `clear_cache` with no argument) every cached entry. `delete`
-  is an alias for `clear_cache(key)`.
+- `clear_cache(key=None)`: Remove a cache entry, or all entries if `key`
+  is omitted. There is no `delete()` method.
 - `get_cached_data_with_strategy(key, data_type)`: Cache get with
   data-type-aware TTL strategy
 - `get_background_cached_data(key, sport_key)`: Cache get for the

docs/DEVELOPER_QUICK_REFERENCE.md

@@ -62,7 +62,7 @@ display_manager.defer_update(lambda: self.update_cache(), priority=0)
 # Basic caching
 cached = cache_manager.get("key", max_age=3600)
 cache_manager.set("key", data)
-cache_manager.delete("key")       # alias for clear_cache(key)
+cache_manager.clear_cache("key")  # there is no delete() method
 
 # Advanced caching
 data = cache_manager.get_cached_data_with_strategy("key", data_type="weather")

docs/FONT_MANAGER.md

@@ -138,28 +138,29 @@ font = self.font_manager.resolve_font(
 
 ## For Plugin Developers
 
-> **Note**: plugins that ship their own fonts via a `"fonts"` block
-> in `manifest.json` are registered automatically during plugin load
-> (`src/plugin_system/plugin_manager.py` calls
-> `FontManager.register_plugin_fonts()`). The `plugin://…` source
-> URIs documented below are resolved relative to the plugin's
-> install directory.
+> ⚠️ **Status**: the plugin-font registration described below is
+> implemented in `src/font_manager.py:150` (`register_plugin_fonts()`)
+> but is **not currently wired into the plugin loader**. Adding a
+> `"fonts"` block to your plugin's `manifest.json` will silently have
+> no effect — the FontManager method exists but nothing calls it.
 >
-> The **Fonts** tab in the web UI that lists detected
-> manager-registered fonts is still a **placeholder
-> implementation** — fonts that managers register through
-> `register_manager_font()` do not yet appear there. The
-> programmatic per-element override workflow described in
-> [Manual Font Overrides](#manual-font-overrides) below
-> (`set_override()` / `remove_override()` / the
-> `config/font_overrides.json` store) **does** work today and is
-> the supported way to override a font for an element until the
-> Fonts tab is wired up. If you can't wait and need a workaround
-> right now, you can also just load the font directly with PIL
-> (or `freetype-py` for BDF) inside your plugin's `manager.py`
-> and skip the override system entirely.
+> Until that's connected, plugin authors who need a custom font
+> should load it directly with PIL (or `freetype-py` for BDF) in
+> their plugin's `manager.py` — `FontManager.resolve_font(family=…,
+> size_px=…)` takes a **family name**, not a file path, so it can't
+> be used to pull a font from your plugin directory. The
+> `plugin://…` source URIs described below are only honored by
+> `register_plugin_fonts()` itself, which isn't wired up.
+>
+> The `/api/v3/fonts/overrides` endpoints and the **Fonts** tab in
+> the web UI are currently **placeholder implementations** — they
+> return empty arrays and contain "would integrate with the actual
+> font system" comments. Manually registered manager fonts do
+> **not** yet flow into that tab. If you need an override today,
+> load the font directly in your plugin and skip the
+> override system.
 
-### Plugin Font Registration
+### Plugin Font Registration (planned)
 
 In your plugin's `manifest.json`:
 

docs/HOW_TO_RUN_TESTS.md

@@ -336,15 +336,15 @@ pytest --cov=src --cov-report=html
 
 ## Continuous Integration
 
-The repo runs
-[`.github/workflows/security-audit.yml`](../.github/workflows/security-audit.yml)
-(bandit + semgrep) on every push. A pytest CI workflow at
-`.github/workflows/tests.yml` is queued to land alongside this
-PR ([ChuckBuilds/LEDMatrix#307](https://github.com/ChuckBuilds/LEDMatrix/pull/307));
-the workflow file itself was held back from that PR because the
-push token lacked the GitHub `workflow` scope, so it needs to be
-committed separately by a maintainer. Once it's in, this section
-will be updated to describe what the job runs.
+There is currently no CI test workflow in this repo — `pytest` runs
+locally but is not gated on PRs. The only GitHub Actions workflow is
+[`.github/workflows/security-audit.yml`](../.github/workflows/security-audit.yml),
+which runs bandit and semgrep on every push.
+
+If you'd like to add a test workflow, the recommended setup is a
+`.github/workflows/tests.yml` that runs `pytest` against the
+supported Python versions (3.10, 3.11, 3.12, 3.13 per
+`requirements.txt`). Open an issue or PR if you want to contribute it.
 
 ## Best Practices
 

docs/PLUGIN_CUSTOM_ICONS.md

@@ -1,5 +1,16 @@
 # Plugin Custom Icons Guide
 
+> ⚠️ **Status:** the `icon` field in `manifest.json` is currently
+> **not honored by the v3 web interface**. Plugin tab icons are
+> hardcoded to `fas fa-puzzle-piece` in
+> `web_interface/templates/v3/base.html:515` and `:774`. The icon
+> field was originally read by a `getPluginIcon()` helper in the v2
+> templates, but that helper wasn't ported to v3. Setting `icon` in a
+> manifest is harmless (it's just ignored) so plugin authors can leave
+> it in place for when this regression is fixed.
+>
+> Tracking issue: see the LEDMatrix repo for the open ticket.
+
 ## Overview
 
 Plugins can specify custom icons that appear next to their name in the web interface tabs. This makes your plugin instantly recognizable and adds visual polish to the UI.

docs/PLUGIN_CUSTOM_ICONS_FEATURE.md

@@ -1,12 +1,13 @@
 # Plugin Custom Icons Feature
 
-> **Note:** this doc was originally written against the v2 web
-> interface. The v3 web interface now honors the same `icon` field
-> in `manifest.json` — the API passes it through at
-> `web_interface/blueprints/api_v3.py` and the three plugin-tab
-> render sites in `web_interface/templates/v3/base.html` read it
-> with a `fas fa-puzzle-piece` fallback. The guidance below still
-> applies; only the referenced template/helper names differ.
+> ⚠️ **Status:** this doc describes the v2 web interface
+> implementation of plugin custom icons. The feature **regressed when
+> the v3 web interface was built** — the `getPluginIcon()` helper
+> referenced below lived in `templates/index_v2.html` (which is now
+> archived) and was not ported to the v3 templates. Plugin tab icons
+> in v3 are hardcoded to `fas fa-puzzle-piece`
+> (`web_interface/templates/v3/base.html:515` and `:774`). The
+> `icon` field in `manifest.json` is currently silently ignored.
 
 ## What Was Implemented
 

plugin-repos/web-ui-info/manager.py

@@ -35,24 +35,24 @@ class WebUIInfoPlugin(BasePlugin):
         """Initialize the Web UI Info plugin."""
         super().__init__(plugin_id, config, display_manager, cache_manager, plugin_manager)
         
-        # AP mode cache (must be initialized before _get_local_ip)
-        self._ap_mode_cached = False
-        self._ap_mode_cache_time = 0.0
-        self._ap_mode_cache_ttl = 60.0
-
         # Get device hostname
         try:
             self.device_id = socket.gethostname()
         except Exception as e:
             self.logger.warning(f"Could not get hostname: {e}, using 'localhost'")
             self.device_id = "localhost"
-
+        
         # Get device IP address
         self.device_ip = self._get_local_ip()
-
+        
         # IP refresh tracking
         self.last_ip_refresh = time.time()
-        self.ip_refresh_interval = 300.0
+        self.ip_refresh_interval = 300.0  # Refresh IP every 5 minutes
+
+        # AP mode cache
+        self._ap_mode_cached = False
+        self._ap_mode_cache_time = 0.0
+        self._ap_mode_cache_ttl = 60.0  # Cache AP mode check for 60 seconds
 
         # Rotation state
         self.current_display_mode = "hostname"  # "hostname" or "ip"
@@ -200,7 +200,9 @@ class WebUIInfoPlugin(BasePlugin):
                                 elif current_interface == "wlan0":
                                     self.logger.debug(f"Found WiFi IP: {ip} on {current_interface}")
                                     return ip
-
+            except Exception:
+                pass
+            
             # Last resort: try hostname resolution (often returns 127.0.0.1)
             try:
                 ip = socket.gethostbyname(socket.gethostname())

scripts/dev/dev_plugin_setup.sh

@@ -6,7 +6,7 @@
 set -euo pipefail
 
 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
-PROJECT_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)"
+PROJECT_ROOT="$SCRIPT_DIR"
 PLUGINS_DIR="$PROJECT_ROOT/plugins"
 CONFIG_FILE="$PROJECT_ROOT/dev_plugins.json"
 DEFAULT_DEV_DIR="$HOME/.ledmatrix-dev-plugins"

scripts/dev/plugins/of-the-day

@@ -0,0 +1 @@
+/home/chuck/.ledmatrix-dev-plugins/ledmatrix-of-the-day

src/base_odds_manager.py

@@ -19,6 +19,14 @@ from datetime import datetime, timedelta, timezone
 from typing import Dict, Any, Optional, List
 import pytz
 
+# Import the API counter function from web interface
+try:
+    from web_interface_v2 import increment_api_counter
+except ImportError:
+    # Fallback if web interface is not available
+    def increment_api_counter(kind: str, count: int = 1):
+        pass
+
 
 class BaseOddsManager:
     """
@@ -123,7 +131,9 @@ class BaseOddsManager:
             response = requests.get(url, timeout=self.request_timeout)
             response.raise_for_status()
             raw_data = response.json()
-
+            
+            # Increment API counter for odds data
+            increment_api_counter('odds', 1)
             self.logger.debug(f"Received raw odds data from ESPN: {json.dumps(raw_data, indent=2)}")
             
             odds_data = self._extract_espn_data(raw_data)

src/cache_manager.py

@@ -320,43 +320,18 @@ class CacheManager:
         return None
 
     def clear_cache(self, key: Optional[str] = None) -> None:
-        """Clear cache entries.
-
-        Pass a non-empty ``key`` to remove a single entry, or pass
-        ``None`` (the default) to clear every cached entry. An empty
-        string is rejected to prevent accidental whole-cache wipes
-        from callers that pass through unvalidated input.
-        """
-        if key is None:
+        """Clear cache for a specific key or all keys."""
+        if key:
+            # Clear specific key
+            self._memory_cache_component.clear(key)
+            self._disk_cache_component.clear(key)
+            self.logger.info("Cleared cache for key: %s", key)
+        else:
             # Clear all keys
             memory_count = self._memory_cache_component.size()
             self._memory_cache_component.clear()
             self._disk_cache_component.clear()
             self.logger.info("Cleared all cache: %d memory entries", memory_count)
-            return
-
-        if not isinstance(key, str) or not key:
-            raise ValueError(
-                "clear_cache(key) requires a non-empty string; "
-                "pass key=None to clear all entries"
-            )
-
-        # Clear specific key
-        self._memory_cache_component.clear(key)
-        self._disk_cache_component.clear(key)
-        self.logger.info("Cleared cache for key: %s", key)
-
-    def delete(self, key: str) -> None:
-        """Remove a single cache entry.
-
-        Thin wrapper around :meth:`clear_cache` that **requires** a
-        non-empty string key — unlike ``clear_cache(None)`` it never
-        wipes every entry. Raises ``ValueError`` on ``None`` or an
-        empty string.
-        """
-        if key is None or not isinstance(key, str) or not key:
-            raise ValueError("delete(key) requires a non-empty string key")
-        self.clear_cache(key)
 
     def list_cache_files(self) -> List[Dict[str, Any]]:
         """List all cache files with metadata (key, age, size, path).

src/logo_downloader.py

@@ -43,9 +43,6 @@ class LogoDownloader:
         'ncaaw': 'https://site.api.espn.com/apis/site/v2/sports/basketball/womens-college-basketball/teams',  # Alias for basketball plugin
         'ncaa_baseball': 'https://site.api.espn.com/apis/site/v2/sports/baseball/college-baseball/teams',
         'ncaam_hockey': 'https://site.api.espn.com/apis/site/v2/sports/hockey/mens-college-hockey/teams',
-        'ncaaw_hockey': 'https://site.api.espn.com/apis/site/v2/sports/hockey/womens-college-hockey/teams',
-        'ncaam_lacrosse': 'https://site.api.espn.com/apis/site/v2/sports/lacrosse/mens-college-lacrosse/teams',
-        'ncaaw_lacrosse': 'https://site.api.espn.com/apis/site/v2/sports/lacrosse/womens-college-lacrosse/teams',
         # Soccer leagues
         'soccer_eng.1': 'https://site.api.espn.com/apis/site/v2/sports/soccer/eng.1/teams',
         'soccer_esp.1': 'https://site.api.espn.com/apis/site/v2/sports/soccer/esp.1/teams',
@@ -76,8 +73,6 @@ class LogoDownloader:
         'ncaa_baseball': 'assets/sports/ncaa_logos',
         'ncaam_hockey': 'assets/sports/ncaa_logos',
         'ncaaw_hockey': 'assets/sports/ncaa_logos',
-        'ncaam_lacrosse': 'assets/sports/ncaa_logos',
-        'ncaaw_lacrosse': 'assets/sports/ncaa_logos',
         # Soccer leagues - all use the same soccer_logos directory
         'soccer_eng.1': 'assets/sports/soccer_logos',
         'soccer_esp.1': 'assets/sports/soccer_logos',

src/plugin_system/plugin_manager.py

@@ -358,23 +358,7 @@ class PluginManager:
             
             # Store module
             self.plugin_modules[plugin_id] = module
-
-            # Register plugin-shipped fonts with the FontManager (if any).
-            # Plugin manifests can declare a "fonts" block that ships custom
-            # fonts with the plugin; FontManager.register_plugin_fonts handles
-            # the actual loading. Wired here so manifest declarations take
-            # effect without requiring plugin code changes.
-            font_manifest = manifest.get('fonts')
-            if font_manifest and self.font_manager is not None and hasattr(
-                self.font_manager, 'register_plugin_fonts'
-            ):
-                try:
-                    self.font_manager.register_plugin_fonts(plugin_id, font_manifest)
-                except Exception as e:
-                    self.logger.warning(
-                        "Failed to register fonts for plugin %s: %s", plugin_id, e
-                    )
-
+            
             # Validate configuration
             if hasattr(plugin_instance, 'validate_config'):
                 try:

web_interface/blueprints/api_v3.py

@@ -1824,7 +1824,6 @@ def get_installed_plugins():
                 'category': plugin_info.get('category', 'General'),
                 'description': plugin_info.get('description', 'No description available'),
                 'tags': plugin_info.get('tags', []),
-                'icon': plugin_info.get('icon', 'fas fa-puzzle-piece'),
                 'enabled': enabled,
                 'verified': verified,
                 'loaded': plugin_info.get('loaded', False),

web_interface/templates/v3/base.html

@@ -512,8 +512,7 @@
                             }
                         }
                     };
-                    const iconClass = (plugin.icon || 'fas fa-puzzle-piece').replace(/"/g, '"');
-                    tabButton.innerHTML = `<i class="${iconClass}"></i>${(plugin.name || plugin.id).replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;')}`;
+                    tabButton.innerHTML = `<i class="fas fa-puzzle-piece"></i>${(plugin.name || plugin.id).replace(/&/g, '&amp;').replace(/</g, '&lt;').replace(/>/g, '&gt;')}`;
                     pluginTabsNav.appendChild(tabButton);
                 });
                 console.log('[GLOBAL] Updated plugin tabs directly:', plugins.length, 'tabs added');
@@ -772,8 +771,7 @@
                                 };
                                 const div = document.createElement('div');
                                 div.textContent = plugin.name || plugin.id;
-                                const iconClass = (plugin.icon || 'fas fa-puzzle-piece').replace(/"/g, '&quot;');
-                                tabButton.innerHTML = `<i class="${iconClass}"></i>${div.innerHTML}`;
+                                tabButton.innerHTML = `<i class="fas fa-puzzle-piece"></i>${div.innerHTML}`;
                                 pluginTabsNav.appendChild(tabButton);
                             });
                             console.log('[STUB] updatePluginTabs: Added', this.installedPlugins.length, 'plugin tabs');
@@ -1961,15 +1959,9 @@
                                 this.updatePluginTabStates();
                             }
                         };
-                        // Build the <i class="..."> + label as DOM nodes so a
-                        // hostile plugin.icon (e.g. containing a quote) can't
-                        // break out of the attribute. escapeHtml only escapes
-                        // <, >, &, not ", so attribute-context interpolation
-                        // would be unsafe.
-                        const iconEl = document.createElement('i');
-                        iconEl.className = plugin.icon || 'fas fa-puzzle-piece';
-                        const labelNode = document.createTextNode(plugin.name || plugin.id);
-                        tabButton.replaceChildren(iconEl, labelNode);
+                        tabButton.innerHTML = `
+                            <i class="fas fa-puzzle-piece"></i>${this.escapeHtml(plugin.name || plugin.id)}
+                        `;
 
                         // Insert before the closing </nav> tag
                         pluginTabsNav.appendChild(tabButton);