mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-06 03:08:05 +00:00
Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
ac44b5a6f5 | ||
|
|
df8f69af30 | ||
|
|
d305be6089 | ||
|
|
53af53b4a1 |
@@ -79,4 +79,5 @@ jobs:
|
|||||||
test/test_sports_scroll.py \
|
test/test_sports_scroll.py \
|
||||||
test/test_version_consistency.py \
|
test/test_version_consistency.py \
|
||||||
test/test_plugin_compatibility_gate.py \
|
test/test_plugin_compatibility_gate.py \
|
||||||
test/test_install_preserves_existing.py
|
test/test_install_preserves_existing.py \
|
||||||
|
test/test_core_owned_config_keys.py
|
||||||
|
|||||||
+116
-33
@@ -212,9 +212,9 @@ one of them is safe by construction and the other is not.
|
|||||||
| **B1** | Promote the nine universal methods; convert `sports.py` → package | ✅ | Characterization suite green; no behavior change intended |
|
| **B1** | Promote the nine universal methods; convert `sports.py` → package | ✅ | Characterization suite green; no behavior change intended |
|
||||||
| **B2** | `CelebrationMixin` + rotation strategies as opt-in capabilities | ✅ | Non-adopters have zero new code in their MRO; strategies checked against verbatim plugin transcriptions |
|
| **B2** | `CelebrationMixin` + rotation strategies as opt-in capabilities | ✅ | Non-adopters have zero new code in their MRO; strategies checked against verbatim plugin transcriptions |
|
||||||
| **B3** | Upstream the scroll **orchestration** layer as `src/common/sports_scroll.py`, reading `global_config['target_fps']` natively | ✅ | Content building stays per-sport |
|
| **B3** | Upstream the scroll **orchestration** layer as `src/common/sports_scroll.py`, reading `global_config['target_fps']` natively | ✅ | Content building stays per-sport |
|
||||||
| **B4** | Ship 3.2.0 *and* make version reporting trustworthy | ⏳ **next** | Tag, release, and `src.__version__` agree; compatibility gate merged |
|
| **B4** | Ship 3.2.0 *and* make version reporting trustworthy | ✅ | Released 2026-08-03; tag, release and `src.__version__` agree; compatibility gate merged (#428, #431, #433) |
|
||||||
| **B5** | Adoption — guarded core imports: three pilots, then the remaining six. **Bundled copies stay.** | after B4 | Per plugin: harness + goldens byte-identical, then a device soak |
|
| **B5** | Adoption — guarded core imports, all eight. **Bundled copies stay.** | ✅ | All eight adopted; harness byte-identical; see the B5 retrospective below — four shipped broken and were repaired in plugins #251 |
|
||||||
| **B6** | Sunset — delete the bundled copies | **blocked** | B4's gate shipped *and* in users' hands (see below) |
|
| **B6** | Sunset — delete the bundled copies | **blocked, deliberately** | 3.2.0 *in users' hands*. Released 2026-08-03; there is no adoption data yet. See "B6 — the decision as of 2026-08-05" |
|
||||||
|
|
||||||
### B4 — what "ship 3.2.0" actually requires
|
### B4 — what "ship 3.2.0" actually requires
|
||||||
|
|
||||||
@@ -265,10 +265,13 @@ migrate there.)
|
|||||||
### B5 — adoption is safe by construction
|
### B5 — adoption is safe by construction
|
||||||
|
|
||||||
A plugin adopting core imports keeps its bundled copy and reaches it through the
|
A plugin adopting core imports keeps its bundled copy and reaches it through the
|
||||||
guarded import (see the Upgradability table above). On a core that ships the
|
guarded import (see the Upgradability table above). On a core that doesn't ship
|
||||||
module the plugin uses core code; on one that doesn't it falls back and behaves
|
the module the plugin falls back and behaves exactly as it does today. That
|
||||||
exactly as it does today. There is no version of this step that breaks a user,
|
fallback compatibility is safe by construction. On a core that *does* ship the
|
||||||
which is why it does not wait for B6's gate.
|
module, correctness is not automatic — object-level and scroll-mode validation
|
||||||
|
(building both classes and comparing, per the retrospective below) is required
|
||||||
|
to prove full behavior. There is no version of this step that breaks a user *on
|
||||||
|
an old core*, which is why it does not wait for B6's gate.
|
||||||
|
|
||||||
The hockey scroll-display pilot is **already validated**: adopted against a core
|
The hockey scroll-display pilot is **already validated**: adopted against a core
|
||||||
carrying 3.2.0, `scroll_display.py` went from 691 to 289 lines and all 16 harness
|
carrying 3.2.0, `scroll_display.py` went from 691 to 289 lines and all 16 harness
|
||||||
@@ -321,35 +324,115 @@ gate rather than trusting the failure to be noticed.
|
|||||||
The same suite should exercise the install/update gate, since it is the other
|
The same suite should exercise the install/update gate, since it is the other
|
||||||
half of the guarantee.
|
half of the guarantee.
|
||||||
|
|
||||||
|
### B6 — the decision as of 2026-08-05
|
||||||
|
|
||||||
|
**Do not run B6 yet. Do not abandon it either.** The blocker is no longer
|
||||||
|
technical; it is calendar time, and it is the one thing here that cannot be
|
||||||
|
worked around by writing more code.
|
||||||
|
|
||||||
|
**Why not yet.** 3.2.0 was published **2026-08-03**. Its predecessor 3.1.0 ran
|
||||||
|
for nine months. B6's entire safety argument is "cores without
|
||||||
|
`src.common.sports_scroll` are gone", and two days after release that is not
|
||||||
|
close to true. There are no release assets to count and no install telemetry, so
|
||||||
|
we cannot demonstrate otherwise — and that absence of evidence *is* the answer.
|
||||||
|
Executing B6 now would strand essentially the whole user base on their current
|
||||||
|
plugin versions.
|
||||||
|
|
||||||
|
**What is already done and waiting.** The hard part is built and tested. The
|
||||||
|
install gate refuses a plugin whose floor exceeds the core's version, and
|
||||||
|
refuses one whose floor is above 2.0.0 when the core reports an untrustworthy
|
||||||
|
version — so a v3.1.0-release user (who reports `1.0.0`) keeps a working plugin
|
||||||
|
instead of receiving one that cannot load. Every adopted plugin has a
|
||||||
|
`test_core_fallback.py` covering both paths.
|
||||||
|
|
||||||
|
**What would unblock it.** Evidence of 3.2.0 uptake — a few months of it being
|
||||||
|
the default download, or store-side install data if that is ever added. Revisit
|
||||||
|
then, not on a schedule.
|
||||||
|
|
||||||
|
**When it happens, remember:** four plugins declare their floor top-level, where
|
||||||
|
editing `versions[0]` is a silent no-op, and the floor has three live spellings
|
||||||
|
(`min_ledmatrix_version`, `requires.min_ledmatrix_version`,
|
||||||
|
`versions[].ledmatrix_min_version`, plus deprecated `ledmatrix_min`). See
|
||||||
|
`src/plugin_system/compatibility.py:declared_min_version` for the resolution
|
||||||
|
order any floor-raising tool must reproduce.
|
||||||
|
|
||||||
|
### B5 retrospective — what the adoption actually cost
|
||||||
|
|
||||||
|
Recorded because it is the evidence behind the two decisions above, and because
|
||||||
|
"the adoption went fine" is not what happened.
|
||||||
|
|
||||||
|
**Four of the eight shipped with scroll mode broken** on a 3.2.0 core, and were
|
||||||
|
repaired in plugins-repo #251. The restructure lifted the content methods
|
||||||
|
verbatim but left the state they read off `self` behind: separator-icon
|
||||||
|
constants (hockey, basketball, lacrosse) and the game-renderer cache (afl).
|
||||||
|
hockey/basketball/lacrosse could not construct the scroll display at all; afl
|
||||||
|
raised inside `prepare_scroll_content`, which the core base *catches*, so its
|
||||||
|
only symptom was scroll mode silently drawing nothing.
|
||||||
|
|
||||||
|
Three things are worth carrying forward:
|
||||||
|
|
||||||
|
- **The bundled fallback did not protect anyone from this.** The break was on
|
||||||
|
the modern path, which the fallback never touches. Carrying the second copy
|
||||||
|
bought nothing against the actual defect while creating the divergence that
|
||||||
|
produced it. That is an argument *for* B6, not against it.
|
||||||
|
- **Every gate was green.** The safety harness renders the scoreboard screens,
|
||||||
|
not scroll mode; `test_core_fallback.py` checked that methods existed and that
|
||||||
|
their *globals* resolved, and `self.NHL_SEPARATOR_ICON` is an attribute read,
|
||||||
|
invisible to an AST scan for `Name` loads. The fix was to stop reasoning about
|
||||||
|
source and **build the object**: construct both classes on both paths, compare
|
||||||
|
the separator icons they end up with, and assert the adopted class ends up
|
||||||
|
with every instance attribute the bundled one sets.
|
||||||
|
- **Test what the change touches, not what is convenient to render.** Scroll
|
||||||
|
mode had no coverage because the harness could not reach it. A comparison
|
||||||
|
harness that renders the same games through both paths and diffs the pixels
|
||||||
|
needs no per-sport knowledge of the right answer, only that adopting core code
|
||||||
|
did not change it.
|
||||||
|
|
||||||
|
**The ledger.** Before adoption, eight duplicated copies totalled 5,685 lines.
|
||||||
|
After adoption plus the frozen legacy copies it was 10,610; removing the dead
|
||||||
|
inline duplication (plugins #252) brought it to roughly 8,620. B6 would take it
|
||||||
|
to about 3,300 including the shared core module — some 2,400 fewer than before
|
||||||
|
this project started. **Until B6 runs, the adoption is net negative on disk**,
|
||||||
|
and its one delivered user-visible gain is that adopted plugins honour the
|
||||||
|
global `target_fps` instead of hardcoding ~100 FPS.
|
||||||
|
|
||||||
|
### Decision: stop adopting further modules until B6 closes
|
||||||
|
|
||||||
|
`data_sources.py` (9 copies), `game_renderer.py` (8) and `base_odds_manager.py`
|
||||||
|
are the obvious next candidates. **Do not adopt them yet.** Each adoption adds
|
||||||
|
carrying cost — a second copy to keep in step — against a payoff that is
|
||||||
|
contingent on B6, and B6 is gated on an installed base we cannot currently
|
||||||
|
measure. Consolidate what is already committed; revisit when B6 does.
|
||||||
|
|
||||||
## What's next
|
## What's next
|
||||||
|
|
||||||
In order. Each step is independently useful and independently revertible.
|
Steps 1–5 of the original plan are **done**: 3.2.0 is tagged and published with
|
||||||
|
a version number CI now asserts (#428), the compatibility gate is in
|
||||||
|
`install_plugin` and reads `compatible_versions` as well as the floor (#431,
|
||||||
|
#433), the newest manifest entry is required to use `ledmatrix_min_version`
|
||||||
|
(plugins #244), and all eight plugins have adopted the scroll orchestration
|
||||||
|
(plugins #245–#249, repaired in #251, tidied in #252).
|
||||||
|
|
||||||
1. **Tag and publish v3.2.0.** The code is already on `main` (`21825cbf`).
|
What actually remains, smallest first:
|
||||||
Nothing else blocks this, and it is what makes `ledmatrix_min_version:
|
|
||||||
"3.2.0"` refer to something real.
|
1. **Nothing on the critical path.** B6 is the only remaining phase and it is
|
||||||
2. **Make the version number honest.** Have the release process assert that the
|
waiting on calendar time, not on work. Resist the urge to fill the gap by
|
||||||
tag, the GitHub release, and `src.__version__` agree — a check in CI is
|
adopting more modules — see the decision above.
|
||||||
cheaper than the confusion of the last two releases. Then revisit the
|
2. **The stale plugin-test tranche** — 5 failures across baseball, hockey and
|
||||||
`< 2.0.0` skip in `_warn_if_incompatible`, which currently silences the
|
basketball, all pre-existing API drift in the plugins' own older tests
|
||||||
warning for the users who most need it.
|
(`plugin.initialized`, `CacheManager(config_manager=...)`, a bare
|
||||||
3. **Add the compatibility gate** to `StoreManager.install_plugin` and
|
`cache_manager` import, `MockLogger.setLevel`, `BasketballPluginManager`).
|
||||||
`.update_plugin`: refuse a plugin whose declared floor exceeds
|
None are scroll-related. They make the suite noisy, which is how a real
|
||||||
`src.__version__`, and surface the reason in the store UI rather than only
|
failure gets ignored.
|
||||||
the log. This is the single change that turns the floor from documentation
|
3. **Soak the remaining adoptions on hardware.** Only baseball has been watched
|
||||||
into a guarantee, and B6 depends on it.
|
through a live game, and hockey has been loaded on devpi. The other six are
|
||||||
4. **Migrate the manifests** to `ledmatrix_min_version`, and reconcile them with
|
proven by harness, unit tests and pixel comparison — not by a live match.
|
||||||
`compatible_versions` (see above — that field is the required, canonical one,
|
Out-of-season sports cannot be soaked until their season starts.
|
||||||
and the gate does not read it yet). Currently 28 plugins spell the floor both
|
4. **`CLAUDE.md` in the plugins repo says four panel sizes; the harness renders
|
||||||
ways across their `versions[]` entries, 12 use only the old spelling, and 2
|
eight.** A one-line doc fix, and the discrepancy has already produced one
|
||||||
only the new. Scope the sweep to the nine sports plugins if a 42-plugin
|
false review finding.
|
||||||
version-bump wave isn't worth it — but the `compatible_versions` half has to
|
5. **Then, when the evidence supports it, B6** — with the four-case
|
||||||
cover every manifest the gate can refuse, or define explicit legacy handling,
|
compatibility regression test above in CI first.
|
||||||
before the gate is allowed to block anything.
|
|
||||||
5. **Run B5 adoption** — hockey, soccer, football, then the remaining six.
|
|
||||||
Bundled copies stay. Byte-identical harness output per plugin, then a soak.
|
|
||||||
6. **Only then plan B6**, with the compatibility regression test described above
|
|
||||||
in CI first.
|
|
||||||
|
|
||||||
## How to keep this project healthy
|
## How to keep this project healthy
|
||||||
|
|
||||||
|
|||||||
@@ -395,6 +395,37 @@ class PluginManager:
|
|||||||
self.state_manager.set_state(plugin_id, PluginState.ERROR, error=e)
|
self.state_manager.set_state(plugin_id, PluginState.ERROR, error=e)
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
#: Config keys the **core** reads out of a plugin's own config block. The
|
||||||
|
#: plugin never declares them, so a schema with
|
||||||
|
#: ``"additionalProperties": false`` — 37 of the 42 published ones — reports
|
||||||
|
#: them as violations and the plugin gets flagged degraded in the web UI for
|
||||||
|
#: using a documented core feature.
|
||||||
|
#:
|
||||||
|
#: Listed explicitly rather than matched on a ``vegas_`` prefix, because
|
||||||
|
#: ``vegas_mode`` is the opposite case: plugins *do* declare that one, and a
|
||||||
|
#: prefix rule would silently stop validating it.
|
||||||
|
#:
|
||||||
|
#: Read by: ``vegas_mode/plugin_adapter.py`` (``vegas_width_pct``,
|
||||||
|
#: ``vegas_overflow``) and ``base_plugin.py`` (``vegas_max_width_screens``).
|
||||||
|
CORE_OWNED_CONFIG_KEYS = frozenset({
|
||||||
|
'vegas_width_pct',
|
||||||
|
'vegas_overflow',
|
||||||
|
'vegas_max_width_screens',
|
||||||
|
})
|
||||||
|
|
||||||
|
def _strip_core_owned_keys(self, config: Dict[str, Any]) -> Dict[str, Any]:
|
||||||
|
"""A shallow copy of ``config`` without the core's own tuning keys.
|
||||||
|
|
||||||
|
Only the top level is touched, and only when such a key is present, so
|
||||||
|
the common case allocates nothing extra.
|
||||||
|
"""
|
||||||
|
if not isinstance(config, dict):
|
||||||
|
return config
|
||||||
|
if not self.CORE_OWNED_CONFIG_KEYS.intersection(config):
|
||||||
|
return config
|
||||||
|
return {k: v for k, v in config.items()
|
||||||
|
if k not in self.CORE_OWNED_CONFIG_KEYS}
|
||||||
|
|
||||||
def _validate_config_schema_soft(self, plugin_id: str, config: Dict[str, Any]) -> None:
|
def _validate_config_schema_soft(self, plugin_id: str, config: Dict[str, Any]) -> None:
|
||||||
"""Validate a plugin's config against its JSON schema — warn/degrade only.
|
"""Validate a plugin's config against its JSON schema — warn/degrade only.
|
||||||
|
|
||||||
@@ -419,7 +450,7 @@ class PluginManager:
|
|||||||
|
|
||||||
try:
|
try:
|
||||||
is_valid, errors = self.schema_manager.validate_config_against_schema(
|
is_valid, errors = self.schema_manager.validate_config_against_schema(
|
||||||
config, schema, plugin_id
|
self._strip_core_owned_keys(config), schema, plugin_id
|
||||||
)
|
)
|
||||||
except Exception as e: # pragma: no cover - defensive
|
except Exception as e: # pragma: no cover - defensive
|
||||||
# Validation machinery itself failed — do not penalise the plugin.
|
# Validation machinery itself failed — do not penalise the plugin.
|
||||||
|
|||||||
@@ -0,0 +1,98 @@
|
|||||||
|
"""The core's own tuning keys must not make a plugin look broken.
|
||||||
|
|
||||||
|
`vegas_width_pct`, `vegas_overflow` and `vegas_max_width_screens` are read by
|
||||||
|
the *core* out of each plugin's config block — `vegas_mode/plugin_adapter.py`
|
||||||
|
and `base_plugin.py`. No plugin declares them, and 37 of the 42 published
|
||||||
|
config schemas set `"additionalProperties": false`, so schema validation
|
||||||
|
reported them as violations.
|
||||||
|
|
||||||
|
That is not just log noise: `_validate_config_schema_soft` sets `degraded` in
|
||||||
|
the health tracker, which the web UI surfaces. Measured on a real device, **9
|
||||||
|
of 27 installed plugins** were flagged degraded purely for using a documented
|
||||||
|
core feature — including `baseball-scoreboard` and `f1-scoreboard`.
|
||||||
|
|
||||||
|
The fix strips those keys before validating. It deliberately does *not* match
|
||||||
|
on a `vegas_` prefix: `vegas_mode` is plugin-owned and declared in schemas, and
|
||||||
|
a prefix rule would silently stop validating it.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from unittest.mock import MagicMock
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
from src.plugin_system.plugin_manager import PluginManager
|
||||||
|
|
||||||
|
|
||||||
|
STRICT_SCHEMA = {
|
||||||
|
"type": "object",
|
||||||
|
"additionalProperties": False,
|
||||||
|
"properties": {
|
||||||
|
"enabled": {"type": "boolean"},
|
||||||
|
"vegas_mode": {"type": "string"}, # plugin-owned, must stay validated
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def manager():
|
||||||
|
mgr = PluginManager.__new__(PluginManager) # skip the heavy constructor
|
||||||
|
mgr.logger = MagicMock()
|
||||||
|
mgr.schema_manager = MagicMock()
|
||||||
|
mgr._set_degraded_safe = MagicMock()
|
||||||
|
return mgr
|
||||||
|
|
||||||
|
|
||||||
|
class TestStripCoreOwnedKeys:
|
||||||
|
def test_removes_every_core_owned_key(self, manager):
|
||||||
|
cfg = {"enabled": True, "vegas_width_pct": 50,
|
||||||
|
"vegas_overflow": "wrap", "vegas_max_width_screens": 2}
|
||||||
|
assert manager._strip_core_owned_keys(cfg) == {"enabled": True}
|
||||||
|
|
||||||
|
def test_leaves_plugin_owned_vegas_mode_alone(self, manager):
|
||||||
|
"""A prefix rule would have eaten this one."""
|
||||||
|
cfg = {"enabled": True, "vegas_mode": "scroll"}
|
||||||
|
assert manager._strip_core_owned_keys(cfg) == cfg
|
||||||
|
|
||||||
|
def test_returns_the_same_object_when_nothing_to_strip(self, manager):
|
||||||
|
cfg = {"enabled": True}
|
||||||
|
assert manager._strip_core_owned_keys(cfg) is cfg
|
||||||
|
|
||||||
|
def test_does_not_mutate_the_caller_config(self, manager):
|
||||||
|
cfg = {"enabled": True, "vegas_width_pct": 50}
|
||||||
|
manager._strip_core_owned_keys(cfg)
|
||||||
|
assert "vegas_width_pct" in cfg, "the live plugin config was mutated"
|
||||||
|
|
||||||
|
def test_tolerates_a_non_dict(self, manager):
|
||||||
|
assert manager._strip_core_owned_keys(None) is None
|
||||||
|
|
||||||
|
|
||||||
|
class TestSoftValidation:
|
||||||
|
def _validate_with(self, manager, config, valid=True, errors=()):
|
||||||
|
manager.schema_manager.load_schema.return_value = STRICT_SCHEMA
|
||||||
|
manager.schema_manager.validate_config_against_schema.return_value = (
|
||||||
|
valid, list(errors))
|
||||||
|
manager._validate_config_schema_soft("baseball-scoreboard", config)
|
||||||
|
return manager.schema_manager.validate_config_against_schema.call_args
|
||||||
|
|
||||||
|
def test_core_keys_never_reach_the_validator(self, manager):
|
||||||
|
"""The regression: these keys reaching a strict schema is what flagged
|
||||||
|
9 of 27 plugins degraded."""
|
||||||
|
args = self._validate_with(
|
||||||
|
manager, {"enabled": True, "vegas_width_pct": 50})
|
||||||
|
validated = args[0][0]
|
||||||
|
assert "vegas_width_pct" not in validated
|
||||||
|
assert validated == {"enabled": True}
|
||||||
|
|
||||||
|
def test_plugin_owned_keys_still_reach_the_validator(self, manager):
|
||||||
|
args = self._validate_with(
|
||||||
|
manager, {"enabled": True, "vegas_mode": "scroll"})
|
||||||
|
assert args[0][0]["vegas_mode"] == "scroll"
|
||||||
|
|
||||||
|
def test_a_genuine_violation_is_still_reported(self, manager):
|
||||||
|
"""Stripping core keys must not turn the check into a no-op."""
|
||||||
|
self._validate_with(
|
||||||
|
manager, {"enabled": True, "typo_key": 1},
|
||||||
|
valid=False, errors=["Field root: 'typo_key' was unexpected"])
|
||||||
|
manager._set_degraded_safe.assert_called()
|
||||||
|
reason = manager._set_degraded_safe.call_args[0][1]
|
||||||
|
assert reason and "typo_key" in reason
|
||||||
Reference in New Issue
Block a user