feat(plugins): apply config edits live without a display restart (4 plugins) - #166
Conversation
|
Warning Review limit reached
Next review available in: 15 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR adds live config reload handling to four plugins, updates their manifests and reported versions, adds regression tests for the new runtime behavior, and refreshes ChangesLive config hot-reload across plugins
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 34 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Four installed plugins had no on_config_change override, so the base BasePlugin only swapped self.config and their derived settings (and the components that captured config at construction) kept startup values until the display service was restarted. This adds on_config_change to each so the 2s config-file watcher applies edits immediately: - ledmatrix-flights: re-derive scalars and rebuild the data-source fetcher, route enrichment and renderer; invalidate the cached map so a new center/radius/zoom re-tiles. - ledmatrix-elections: re-derive filters, rebuild providers + race store, reconfigure the scroll helper, and force a re-fetch. - baseball-scoreboard / football-scoreboard: re-derive league/display settings and rebuild the per-league managers (cleaning up old HTTP sessions first), league registry, scroll manager and rotation modes so favorite teams, league enable/disable, durations and live priority apply live. Per-game progress tracking is reset since manager keys may change. Each plugin gains a regression test that fails against the base behavior and passes with the override. Versions bumped + plugins.json synced. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
6397855 to
16e965b
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/baseball-scoreboard/manager.py`:
- Around line 278-279: The inconsistency between how self.enabled and
self.is_enabled are initialized during config reloads can unintentionally
re-enable runtime behavior on partial config updates. Line 278 correctly
preserves the prior state of self.enabled when the "enabled" key is missing by
using getattr(self, "enabled", True) as a fallback, but line 279 hard-defaults
self.is_enabled to True instead. Fix this by updating line 279 to follow the
same pattern as line 278: use getattr(self, "is_enabled", True) as the fallback
value instead of just True, so that self.is_enabled also preserves its prior
state when the config key is omitted.
In `@plugins/football-scoreboard/manifest.json`:
- Line 4: The version string in the get_info() method in manager.py is out of
sync with the updated version in manifest.json. Update the version value
returned by the get_info() method from "2.1.1" to "2.4.0" to ensure the
runtime-reported plugin version matches the manifest version and maintains
consistency for diagnostics and UI consumers.
In `@plugins/ledmatrix-flights/manager.py`:
- Around line 409-410: The configuration update for tracked_flights_cfg does not
clean up stale entries from the runtime tracked_flight_data dictionary when
flights are removed from the config during hot-reload. After updating
tracked_flights_cfg on line 409, add logic to identify and remove any entries
from tracked_flight_data that no longer exist in the updated configuration by
comparing the tracked flight identifiers between the old and new config values.
🪄 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: 688b6c86-75df-497a-8430-7717abc641cb
📒 Files selected for processing (16)
plugins.jsonplugins/baseball-scoreboard/manager.pyplugins/baseball-scoreboard/manifest.jsonplugins/baseball-scoreboard/test_config_reload.pyplugins/clock-simple/manager.pyplugins/clock-simple/manifest.jsonplugins/clock-simple/test_config_reload.pyplugins/football-scoreboard/manager.pyplugins/football-scoreboard/manifest.jsonplugins/football-scoreboard/test_config_reload.pyplugins/ledmatrix-elections/manager.pyplugins/ledmatrix-elections/manifest.jsonplugins/ledmatrix-elections/test_config_reload.pyplugins/ledmatrix-flights/manager.pyplugins/ledmatrix-flights/manifest.jsonplugins/ledmatrix-flights/test_config_reload.py
- baseball-scoreboard: self.is_enabled was hard-defaulted to True on every on_config_change call, unlike self.enabled which preserves its prior value when the "enabled" key is omitted from the new config. Since is_enabled is the flag actually checked throughout the plugin (enabled is otherwise unused), a partial config save that omits "enabled" would silently re-enable a disabled plugin. Now uses the same getattr(self, "is_enabled", True) fallback as self.enabled. - football-scoreboard: get_info() reported version "2.1.1", stale against manifest.json's "2.4.0". Updated to match. - ledmatrix-flights: removing a flight from tracked_flights left its TrackedFlight entry frozen in tracked_flight_data forever (nothing ever pruned it), so a live-removed flight kept showing up in the tracked list/status checks until a full restart -- defeating the point of this PR for that specific setting. on_config_change now drops runtime tracking state for identifiers no longer in the updated config. Added regression tests for the two behavioral fixes (is_enabled preservation, tracked_flight_data pruning); both verified to fail against the pre-fix code and pass after. Ran each plugin's existing test_config_reload.py suite (9/9, 9/9, 15/15) -- no regressions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
…ions main advanced independently while this branch was open (favorite_live_boost across several sport scoreboards, baseball's pitcher/batter/last-play work elsewhere, new community plugins). Conflicts were all version-number collisions where this branch and main happened to pick the same next version for different features from a shared ancestor point: - baseball-scoreboard: this branch's config-hot-reload bump (1.7.0) collided with main's already-merged favorite_live_boost (also 1.7.0). Renumbered this branch's bump to 1.8.0 (next after main's 1.7.0) and updated get_info()'s hardcoded version to match (it was still at the pre-existing stale 1.6.0). - football-scoreboard: this branch's bump (2.4.0) collided with an already-merged celebration-takeover feature that had also claimed 2.4.0; main's own favorite_live_boost bump had already moved past that to 2.5.0. Renumbered this branch's bump to 2.6.0 and updated get_info() to match. - plugins.json: regenerated via update_registry.py against the resolved manifests rather than hand-merging the generated JSON. Diff is exactly the four plugins this PR bumps (baseball-scoreboard, football-scoreboard, ledmatrix-flights, ledmatrix-elections) plus their last_updated stamps -- hand-reverted two unrelated unicode-escaping changes the sync script makes to on-air/plex-marquee's descriptions on every regen. ledmatrix-flights and ledmatrix-elections had no collisions -- their bumps (1.10.0, 1.1.0) were already ahead of main's versions (1.9.2, 1.0.2) and merged cleanly. Validated: all four plugins compile, their own test_config_reload.py suites pass (12/12, 9/9, 15/15, 9/9), main's favorite_live_boost regression tests for the auto-merged sport scoreboards still pass, module-collision gate passes, both resolved manifests validate against the core schema. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/football-scoreboard/manager.py`:
- Around line 259-260: The config reload logic in FootballScoreboardManager is
resetting `self.is_enabled` to `True` whenever `enabled` is missing, which
reactivates previously disabled plugins. Update the assignment near
`self.enabled`/`self.is_enabled` so `is_enabled` preserves the current state on
reloads by falling back to the existing attribute instead of hardcoding `True`,
keeping the `update`, `display`, and `live` paths consistent with prior
disablement.
In `@plugins/ledmatrix-flights/test_config_reload.py`:
- Around line 34-41: The regression test in test_config_reload.py can appear
green under pytest because check() only updates counters and prints instead of
failing the test. Update check() so a false condition raises an AssertionError
(or equivalent test failure) while keeping the pass counter/logging, and ensure
the __main__ exit path still reflects _failed for direct script execution. Use
the existing check() helper and the __main__ guard around sys.exit to locate the
changes.
- Around line 20-28: The shared test imports are too generic and can resolve to
the wrong plugin module when multiple plugins are loaded in one process. Update
the imports in the test setup around FlightTrackerPlugin and TrackedFlight so
they use plugin-unique helper module names or load those helpers directly by
file path instead of bare manager/data_model imports, ensuring this test cannot
bind to a cached manager.py from another plugin.
🪄 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: fb30842d-8aae-448c-8233-ea433154ae51
📒 Files selected for processing (13)
plugins.jsonplugins/baseball-scoreboard/manager.pyplugins/baseball-scoreboard/manifest.jsonplugins/baseball-scoreboard/test_config_reload.pyplugins/football-scoreboard/manager.pyplugins/football-scoreboard/manifest.jsonplugins/football-scoreboard/test_config_reload.pyplugins/ledmatrix-elections/manager.pyplugins/ledmatrix-elections/manifest.jsonplugins/ledmatrix-elections/test_config_reload.pyplugins/ledmatrix-flights/manager.pyplugins/ledmatrix-flights/manifest.jsonplugins/ledmatrix-flights/test_config_reload.py
✅ Files skipped from review due to trivial changes (4)
- plugins/football-scoreboard/manifest.json
- plugins/baseball-scoreboard/manifest.json
- plugins.json
- plugins/ledmatrix-flights/manifest.json
🚧 Files skipped from review as they are similar to previous changes (5)
- plugins/ledmatrix-elections/test_config_reload.py
- plugins/football-scoreboard/test_config_reload.py
- plugins/baseball-scoreboard/test_config_reload.py
- plugins/ledmatrix-elections/manifest.json
- plugins/ledmatrix-elections/manager.py
…eload PR - football-scoreboard: same self.is_enabled hard-defaulted-to-True bug already fixed in baseball-scoreboard (copy-pasted into this plugin's own on_config_change too). Now preserves prior state via getattr fallback, matching self.enabled. - All four test_config_reload.py files share an identical check() helper that only counts/prints on failure instead of raising -- confirmed this makes pytest report false green: a deliberately-broken check still showed "2 passed" under `pytest test_config_reload.py`. check() now raises AssertionError on failure (still counts/logs first). The __main__ path wraps each test function call in its own try/except so direct script execution still runs every test function and reports an accurate "N passed, M failed" summary with the correct exit code. - ledmatrix-flights/test_config_reload.py's bare `from manager import X` / `from data_model import Y` can silently bind to a *different* plugin's same-named module if one was already cached in sys.modules earlier in the same process (e.g. another plugin's test_config_reload.py ran first under a shared pytest session) -- reproduced this exactly: importing baseball-scoreboard's manager.py first, then running this file's import logic, raised ImportError because sys.modules['manager'] was still baseball's. Now pops any stale manager/data_model entries before importing so this plugin's own files always win. Verified: the false-green pytest behavior is fixed (a broken check now fails the pytest run, confirmed with a deliberately-broken check that previously showed all green); the cross-plugin import collision is fixed (reproduced the ImportError, confirmed it no longer happens); standalone `python test_config_reload.py` execution for all four plugins still exits 0 on success and 1 on a genuine failure with an accurate summary. Ran all four plugins' full suites (12/12, 9/9, 15/15, 9/9) and the module-collision gate -- no regressions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
Found while independently verifying the PR end-to-end (not one of the originally flagged review comments): on_config_change hard-defaulted self.enabled to False on every call, unlike baseball-scoreboard and football-scoreboard's self.enabled (which correctly fall back to the current value via getattr). Currently dormant -- no code path reads plugin.enabled today (PluginManager.get_enabled_plugins() has zero callers) -- but it's part of the documented public plugin API and the exact same bug class already fixed twice elsewhere in this PR, so closing it now rather than leaving a live foot-gun for whenever something does call it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
…on collisions main advanced with PR #166 (config hot-reload for baseball/football/ elections/flights) merging in since this branch was opened. Conflicts were version-number collisions in baseball-scoreboard, where this branch and #166 picked the same next version for different features from a shared ancestor point: - baseball-scoreboard: main's config-hot-reload feature landed as 1.8.0. This branch had independently used 1.8.0 for pitcher/batter/last-play and 1.9.0 for the traditional scoreboard. Renumbered this branch's two bumps to 1.9.0 and 1.10.0 (after main's 1.8.0), keeping main's 1.8.0 entry unchanged, and updated get_info()'s hardcoded version to match. - plugins.json: regenerated via update_registry.py against the resolved manifest rather than hand-merging the generated JSON. Diff is exactly the one version bump this branch changes, plus the usual two unrelated unicode-escaping lines (on-air/plex-marquee) the sync script rewrites on every regen -- reverted those by hand as usual. football-scoreboard/ledmatrix-elections/ledmatrix-flights changes (config hot-reload, from #166) merged cleanly with no conflicts -- unrelated to this branch's work, pulled in as part of catching up to main. Verified the two features now actually compose correctly, not just that they compile: on_config_change (from #166) rebuilds the per-league managers via _initialize_managers() -> _adapt_config_for_manager(), which is exactly where show_pitcher_batter/show_last_play/ show_traditional_scoreboard are read -- so a live toggle of any of this branch's new features takes effect without a restart, the same as every other display option. Ran: all four baseball-scoreboard test suites (test_config_reload.py 12/12, test_pitcher_batter_last_play.py 11/11, test_traditional_scoreboard.py 11/11, test_favorite_live_boost.py all passing), the module-collision gate, and the full safety harness across all 7 sizes x both leagues x every combined toggle -- 42/42 PASS, zero failures. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KEZK1P1Q1fu5pcuVrkrCFZ
Problem
Most plugin settings only took effect after restarting the display service. The core already hot-reloads
config.json(a 2s file-watcher inConfigServicenotifiesBasePlugin.on_config_change), but the baseon_config_changeonly swapsself.config— it doesn't re-derive a plugin's settings, and it can't rebuild the components a plugin builds from config at construction. So editing a setting in the web UI appeared to do nothing until a restart.Most first-party plugins never override
on_config_change. This adds it to four of them so edits apply live.What each plugin now refreshes live
configat construction) and invalidates the cached map background so a new center/radius/zoom re-tiles. Runtime state (tracked aircraft, trails, API counters, records, proximity state machine) is left untouched.Genuinely hardware-level settings (panel geometry, etc.) are intentionally not made live — those still warrant a clean restart.
Tests
Each plugin gets a
test_config_reload.pythat instantiates the plugin with stub managers, callson_config_change, and asserts the derived settings/components update. Each was verified to fail against the base behavior and pass with the override.check_plugin.py) against all four at every size — no crashes/overflow, all PASS.ledmatrix-electionssuites still pass (72 + 61).Notes
Versions bumped (minor) and
plugins.jsonsynced. The same one-method pattern extends to the remaining plugins in follow-ups. (clock-simplewas prepared too but is held back for a separate PR: it has a pre-existing 64×32 / 64×64 overflow — unrelated to config reload — that the safety harness flags, and that should be fixed on its own.)Summary by CodeRabbit