fix: post-audit follow-ups (lacrosse rename + dead v2 imports)#93
Merged
ChuckBuilds merged 1 commit intomainfrom Apr 8, 2026
Merged
fix: post-audit follow-ups (lacrosse rename + dead v2 imports)#93ChuckBuilds merged 1 commit intomainfrom
ChuckBuilds merged 1 commit intomainfrom
Conversation
The docs refresh audit on #92 flagged two code bugs affecting the plugins repo. Neither could be fixed in a docs-only PR, so they were deferred to this follow-up. The companion LEDMatrix PR (ChuckBuilds/LEDMatrix#307) handles the six bugs that belong in the app repo. Bug 6: lacrosse-scoreboard display-mode collision (BREAKING) lacrosse-scoreboard shipped six display modes (ncaa_mens_recent / upcoming / live and ncaa_womens_recent / upcoming / live) that collide with the same six modes exposed by hockey-scoreboard. LEDMatrix's display controller stores modes in a flat dict keyed by mode name (src/display_controller.py), so whichever plugin loaded second silently overrode the first — with no warning in the logs. Renamed all six modes with a plugin-specific lax_ prefix: ncaa_mens_recent -> lax_ncaa_mens_recent ncaa_mens_upcoming -> lax_ncaa_mens_upcoming ncaa_mens_live -> lax_ncaa_mens_live ncaa_womens_recent -> lax_ncaa_womens_recent ncaa_womens_upcoming -> lax_ncaa_womens_upcoming ncaa_womens_live -> lax_ncaa_womens_live Manifest, dispatch logic in manager.py (_get_current_manager, _record_dynamic_progress, get_cycle_duration, and the granular mode-parser that walks the league registry) all updated to use the new names. The split('_', 2)[2] parser has been replaced with rsplit('_', 1)[1] where the new four-segment mode strings (lax_ncaa_mens_recent) need the last segment. README "Known conflict" warning replaced with a breaking-change note pointing at the new CHANGELOG.md, which spells out the rename table and migration steps for anyone with the old mode names hardcoded in their config.json (display_durations, rotation_order, etc.). Version bumped 1.0.3 -> 1.1.0 (minor bump per semver — breaking config change). No backward-compat aliases. Bug 5: dead web_interface_v2 import pattern (plugins half) Every plugin that used to talk to the v2 API-counter helper carried a try/except block importing web_interface_v2 and falling back to a no-op stub. The web_interface_v2 module doesn't exist anywhere in the v3 codebase — the counter was silently a no-op for every plugin, and no API-metrics dashboard reads the counter. Deleted the import block, all direct calls, one self.increment_api_counter hasattr-guarded call site in odds-ticker/data_fetcher.py, and the orphan "# Increment API counter" comment lines left behind. Touched plugins (14 files): - baseball-scoreboard/base_odds_manager.py - basketball-scoreboard/base_odds_manager.py - football-scoreboard/base_odds_manager.py - hockey-scoreboard/base_odds_manager.py - lacrosse-scoreboard/base_odds_manager.py - soccer-scoreboard/base_odds_manager.py - ufc-scoreboard/base_odds_manager.py - ledmatrix-leaderboard/data_fetcher.py - ledmatrix-music/manager.py - ledmatrix-stocks/data_fetcher.py - ledmatrix-weather/manager.py - odds-ticker/manager.py - odds-ticker/data_fetcher.py - youtube-stats/manager.py Verification done locally: - ast.parse on all plugin .py files (0 syntax errors) - py_compile on every touched file - No web_interface_v2 or increment_api_counter references remain anywhere under plugins/ (except the unrelated src.api_counter mock in lacrosse-scoreboard/test_lacrosse_plugin.py) - Lacrosse lax_ prefix applied consistently across manifest, default mode fallback, dispatch logic, and mode-string parser Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR removes Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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 docstrings
🧪 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/lacrosse-scoreboard/manager.py (1)
1439-1468:⚠️ Potential issue | 🔴 Critical
_get_manager_for_modewill returnNonefor modes with thelax_prefix.The method expects unprefixed mode names (e.g.,
ncaa_mens_recent), but callers at lines 1750, 1793, and 1833 pass mode names extracted fromself.modeswhich include thelax_prefix (e.g.,lax_ncaa_mens_recent). Since the method only checksstartswith("ncaa_mens_")andstartswith("ncaa_womens_"), these prefixed modes will not match and the method will returnNone, breaking the dynamic cycle completion logic.Strip the
lax_prefix frommode_nameat the beginning of_get_manager_for_mode, or adjust the caller to remove the prefix before passing it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/lacrosse-scoreboard/manager.py` around lines 1439 - 1468, The _get_manager_for_mode method fails to match mode names that include the "lax_" prefix (e.g., "lax_ncaa_mens_recent") because it only checks startswith("ncaa_mens_") and startswith("ncaa_womens_"); update _get_manager_for_mode to normalize the incoming mode_name by stripping a leading "lax_" prefix (if present) before the existing startswith checks so callers that pass modes from self.modes (like "lax_ncaa_mens_recent") will resolve to the correct manager (ncaa_mens_live, ncaa_mens_recent, ncaa_mens_upcoming, etc.).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@plugins/lacrosse-scoreboard/manager.py`:
- Around line 1439-1468: The _get_manager_for_mode method fails to match mode
names that include the "lax_" prefix (e.g., "lax_ncaa_mens_recent") because it
only checks startswith("ncaa_mens_") and startswith("ncaa_womens_"); update
_get_manager_for_mode to normalize the incoming mode_name by stripping a leading
"lax_" prefix (if present) before the existing startswith checks so callers that
pass modes from self.modes (like "lax_ncaa_mens_recent") will resolve to the
correct manager (ncaa_mens_live, ncaa_mens_recent, ncaa_mens_upcoming, etc.).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4708a2c5-725f-4e93-8b8c-06b2586034cd
📒 Files selected for processing (19)
plugins.jsonplugins/baseball-scoreboard/base_odds_manager.pyplugins/basketball-scoreboard/base_odds_manager.pyplugins/football-scoreboard/base_odds_manager.pyplugins/hockey-scoreboard/base_odds_manager.pyplugins/lacrosse-scoreboard/CHANGELOG.mdplugins/lacrosse-scoreboard/README.mdplugins/lacrosse-scoreboard/base_odds_manager.pyplugins/lacrosse-scoreboard/manager.pyplugins/lacrosse-scoreboard/manifest.jsonplugins/ledmatrix-leaderboard/data_fetcher.pyplugins/ledmatrix-music/manager.pyplugins/ledmatrix-stocks/data_fetcher.pyplugins/ledmatrix-weather/manager.pyplugins/odds-ticker/data_fetcher.pyplugins/odds-ticker/manager.pyplugins/soccer-scoreboard/base_odds_manager.pyplugins/ufc-scoreboard/base_odds_manager.pyplugins/youtube-stats/manager.py
💤 Files with no reviewable changes (12)
- plugins/hockey-scoreboard/base_odds_manager.py
- plugins/youtube-stats/manager.py
- plugins/lacrosse-scoreboard/base_odds_manager.py
- plugins/football-scoreboard/base_odds_manager.py
- plugins/soccer-scoreboard/base_odds_manager.py
- plugins/ledmatrix-stocks/data_fetcher.py
- plugins/ledmatrix-leaderboard/data_fetcher.py
- plugins/ledmatrix-weather/manager.py
- plugins/basketball-scoreboard/base_odds_manager.py
- plugins/baseball-scoreboard/base_odds_manager.py
- plugins/odds-ticker/manager.py
- plugins/ufc-scoreboard/base_odds_manager.py
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
Companion to ChuckBuilds/LEDMatrix#307. Fixes the two code bugs
the docs refresh audit on #92 surfaced that belong in this repo.
Bug 6: lacrosse-scoreboard display-mode collision (BREAKING)
`lacrosse-scoreboard` shipped six display modes
(`ncaa_mens_recent` / `upcoming` / `live` and
`ncaa_womens_*`) that collided with the same six names used by
`hockey-scoreboard`. LEDMatrix's display controller stores modes
in a flat dict keyed by mode name, so whichever plugin loaded
second silently overrode the first — no log warning, no crash,
just silent data loss.
All six lacrosse modes gained a `lax_` prefix:
Updated the manifest, the default-mode fallback in `manager.py`,
the dispatch in `_get_current_manager`, the mode-string parser
(`record_dynamic_progress`, `get_cycle_duration`, and the
granular parser in `display_plugin_mode`), and the
`live_modes.append(...)` literals in `get_live_modes`. The old
`current_mode.split('', 2)[2]` parser assumed two underscores
before the mode-type segment; the new four-segment strings
(`lax_ncaa_mens_recent`) have three, so the parser now uses
`rsplit('', 1)[1]` which is correct regardless of prefix depth.
Version bumped 1.0.3 → 1.1.0 (minor bump per semver for a
breaking config change). README's old "Known conflict" warning
is replaced with a breaking-change notice; full migration table
and instructions live in the new
`CHANGELOG.md`.
No backward-compat aliases — the old mode names are no
longer recognized by the dispatch logic. Anyone with the old
names in `display_durations`, `rotation_order`, or any other
config key needs to migrate per the CHANGELOG.
Bug 5: dead `web_interface_v2` import pattern
14 plugin files carried a `try/except` block importing
`web_interface_v2.increment_api_counter` with a no-op fallback:
```python
try:
from web_interface_v2 import increment_api_counter
except ImportError:
def increment_api_counter(kind: str, count: int = 1):
pass
```
The `web_interface_v2` module doesn't exist anywhere in the v3
codebase — the counter has been silently a no-op for every
plugin, and no API-metrics dashboard reads it. Deleted the
import block, every direct call site, one
`self.increment_api_counter` `hasattr`-guarded call in
`odds-ticker/data_fetcher.py`, and the orphan
`# Increment API counter` comments left behind.
Touched plugins (14 files):
Verification done locally
remain anywhere under `plugins/` (except an unrelated
`src.api_counter` mock in
`lacrosse-scoreboard/test_lacrosse_plugin.py`)
manifest, default mode fallback, dispatch logic, and
mode-string parser
`1.1.0` version
Test plan
side-by-side and confirm both plugins' NCAA modes appear in
the rotation (no silent override).
`ImportError: web_interface_v2` warnings in
`journalctl -u ledmatrix`.
scoreboards and confirm odds still render correctly after
the counter-call removal.
keys in `config.json` under a lacrosse-specific section,
follow the migration steps in the new
`lacrosse-scoreboard/CHANGELOG.md`.
🤖 Generated with Claude Code
Summary by CodeRabbit
Breaking Changes
lax_prefix to all NCAA mode identifiers (e.g.,ncaa_mens_recent→lax_ncaa_mens_recent). Update any references in display settings.Chores