From bb475a79ea8beeba72c0aa4c51974fa65fd9e5be Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Mon, 5 Oct 2026 09:06:56 -0400 Subject: [PATCH 1/8] fix(plugins): keep a plugin's tokens and local files across store updates (#755) * fix(plugins): keep a plugin's tokens and local files across store updates A monorepo plugin update replaces the plugin directory with the fresh download and deletes the old copy, taking with it everything the plugin wrote beside itself. On 2026-10-04 updating calendar 1.2.9 -> 1.2.12 deleted token.pickle and credentials.json, and the calendar stopped until they were restored from a backup. Before the set-aside copy is discarded (store update, reinstall over an existing copy, install_from_url replace), carry over files the plugin's .gitignore excludes plus known secret/state files (*.pickle, token.json, credentials.json, config_secrets.json, .pkce_code_verifier). Files the new release ships win; byte code and .git are not carried; if a copy fails the old copy is kept. The git-pull path no longer sweeps untracked tokens into its auto-stash, which is never popped. Co-Authored-By: Claude Opus 5.5 * fix(plugins): find the new copy via _existing_install, as install_plugin does Co-Authored-By: Claude Opus 5.5 * test(on-demand): find the write under test by key, not by position TestARestoreWithNothingToResume took the last cache_manager.set call to be the on-demand state, but the controller's font-usage publisher thread writes font_usage_snapshot to the same mock, and on a slow runner it lands last. Failing on main since #748 (Python 3.11 job). Same fix for the named-mode restart test, which had the same race. Co-Authored-By: Claude Opus 5.5 * test(starlark): fake only the editor launch, not every Popen in the request TestPixletEditorHostDefaultsButDoesNotOverride patched subprocess.Popen for the whole request. When the captive-portal before_request hook's 30s AP-mode cache had expired, its `systemctl is-active hostapd` check went through subprocess.run, got the fake process, and raised TypeError (run() uses the process as a context manager): a 500 instead of 200. Seen on the Python 3.13 job; reproduced locally by forcing the cache to expire. Other calls now reach the real Popen. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 11 + src/plugin_system/plugin_local_files.py | 204 ++++++++++++++++ src/plugin_system/store_install.py | 37 ++- src/plugin_system/store_update.py | 29 ++- test/test_store_update_keeps_local_files.py | 244 ++++++++++++++++++++ 5 files changed, 516 insertions(+), 9 deletions(-) create mode 100644 src/plugin_system/plugin_local_files.py create mode 100644 test/test_store_update_keeps_local_files.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 71c3fbe6..3ba95dce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1590,6 +1590,17 @@ read any of them: ### Fixes +- Updating a plugin from the store no longer deletes the files it wrote + beside itself. A monorepo update replaces the plugin directory with the + fresh download and deletes the old copy, so calendar's Google OAuth files + (`token.pickle`, `credentials.json`) were lost on every update and the + calendar stopped until they were restored by hand. Before the old copy is + removed, the update now copies over anything the plugin's `.gitignore` + excludes plus known secret/state files (`*.pickle`, `token.json`, + `credentials.json`, `config_secrets.json`, `.pkce_code_verifier`); files the + new release ships are never overwritten, and byte code is not carried. A + plugin updated with `git pull` no longer sweeps an untracked token into the + auto-stash, which is never popped (`src/plugin_system/plugin_local_files.py`). - Quieter routine logging. Every rotation logged each mode twice ("Switching to mode", then "Processing mode"), and a mode with nothing to show added "display() returned False" and "No content to display". Those diff --git a/src/plugin_system/plugin_local_files.py b/src/plugin_system/plugin_local_files.py new file mode 100644 index 00000000..1a3ac2f4 --- /dev/null +++ b/src/plugin_system/plugin_local_files.py @@ -0,0 +1,204 @@ +""" +Files a plugin writes beside itself at runtime, which an update must keep. + +A store update replaces a plugin's directory with a fresh download and then +deletes the old copy. Anything the plugin created there -- OAuth tokens, a +client-secrets file, a PKCE verifier, cached state -- is in no release, so the +fresh download does not contain it and deleting the old copy destroys it. On +2026-10-04 updating calendar 1.2.9 -> 1.2.12 that way deleted its +``token.pickle`` and ``credentials.json``, and the calendar stopped until they +were restored from a backup. + +What counts as "the plugin's own local file" is the union of: + +* :data:`KNOWN_STATE_PATTERNS` -- secret and state files plugins are known to + write, kept even when a plugin forgot to gitignore them; and +* whatever the plugin's own ``.gitignore`` (old copy or new) excludes. A file + the author ignores is by definition not part of a release. + +A file the new release ships is never overwritten: tracked content wins. Byte +code (``__pycache__``, ``*.pyc``) and ``.git`` are never carried, since they +belong to the old code rather than to the user. +""" + +from __future__ import annotations + +import fnmatch +import os +import re +import shutil +from pathlib import Path +from typing import Iterable, List, Optional, Pattern, Tuple + +__all__ = [ + 'KNOWN_STATE_PATTERNS', + 'carry_over_local_files', + 'is_known_state_file', + 'local_files_to_keep', +] + +# Basename globs. Kept even when the plugin's .gitignore does not list them. +KNOWN_STATE_PATTERNS: Tuple[str, ...] = ( + 'token.pickle', + '*.pickle', + 'token.json', + 'credentials.json', + 'config_secrets.json', + '.pkce_code_verifier', +) + +_NEVER_CARRY_DIRS = frozenset({'.git', '__pycache__'}) +_NEVER_CARRY_SUFFIXES = ('.pyc', '.pyo') + + +def is_known_state_file(rel_path: str) -> bool: + """True when ``rel_path``'s basename is a known secret/state file.""" + name = rel_path.replace('\\', '/').rsplit('/', 1)[-1] + return any(fnmatch.fnmatchcase(name, p) for p in KNOWN_STATE_PATTERNS) + + +class _GitIgnore: + """The subset of gitignore semantics plugin .gitignore files use. + + Supports comments, ``!`` negation (last match wins), a trailing ``/`` for + directory-only patterns, anchoring by a leading or embedded ``/``, ``*``, + ``?``, ``[...]`` and ``**``. As in git, a file under an ignored directory + is ignored regardless of later negations. + """ + + def __init__(self, lines: Iterable[str]): + self._rules: List[Tuple[Pattern[str], bool, bool]] = [] + for raw in lines: + line = raw.rstrip('\n').rstrip() + if not line or line.startswith('#'): + continue + negate = line.startswith('!') + if negate: + line = line[1:] + elif line.startswith('\\'): + line = line[1:] + dir_only = line.endswith('/') + line = line.rstrip('/') + if not line: + continue + anchored = '/' in line + line = line.lstrip('/') + body = self._translate(line) + regex = body if anchored else r'(?:.*/)?' + body + self._rules.append((re.compile(r'\A' + regex + r'\Z'), negate, dir_only)) + + @staticmethod + def _translate(pattern: str) -> str: + out, i, n = [], 0, len(pattern) + while i < n: + if pattern.startswith('**/', i): + out.append(r'(?:.*/)?') + i += 3 + elif pattern.startswith('/**', i) and i + 3 == n: + out.append(r'/.*') + i += 3 + elif pattern.startswith('**', i): + out.append(r'.*') + i += 2 + elif pattern[i] == '*': + out.append(r'[^/]*') + i += 1 + elif pattern[i] == '?': + out.append(r'[^/]') + i += 1 + elif pattern[i] == '[': + end = pattern.find(']', i + 1) + if end == -1: + out.append(re.escape('[')) + i += 1 + else: + cls = pattern[i + 1:end] + if cls.startswith('!'): + cls = '^' + cls[1:] + out.append('[' + cls.replace('\\', '\\\\') + ']') + i = end + 1 + else: + out.append(re.escape(pattern[i])) + i += 1 + return ''.join(out) + + def _decide(self, rel: str, is_dir: bool) -> Optional[bool]: + verdict = None + for regex, negate, dir_only in self._rules: + if dir_only and not is_dir: + continue + if regex.match(rel): + verdict = not negate + return verdict + + def ignores(self, rel_path: str) -> bool: + if not self._rules: + return False + parts = rel_path.replace('\\', '/').split('/') + for depth in range(1, len(parts)): + if self._decide('/'.join(parts[:depth]), True): + return True + return bool(self._decide('/'.join(parts), False)) + + +def _read_gitignore(plugin_dir: Path) -> List[str]: + try: + return (plugin_dir / '.gitignore').read_text( + encoding='utf-8', errors='replace').splitlines() + except OSError: + return [] + + +def local_files_to_keep(old_dir: Path, new_dir: Path) -> List[str]: + """Relative paths (``/``-separated) in ``old_dir`` to copy into ``new_dir``. + + Regular files only; symlinks and anything the new release already ships + are skipped. + """ + old_dir, new_dir = Path(old_dir), Path(new_dir) + ignore = _GitIgnore(_read_gitignore(old_dir) + _read_gitignore(new_dir)) + keep: List[str] = [] + for root, dirs, files in os.walk(old_dir): + dirs[:] = sorted(d for d in dirs if d not in _NEVER_CARRY_DIRS + and not os.path.islink(os.path.join(root, d))) + rel_root = os.path.relpath(root, old_dir) + for name in sorted(files): + if name.endswith(_NEVER_CARRY_SUFFIXES): + continue + full = os.path.join(root, name) + if os.path.islink(full) or not os.path.isfile(full): + continue + rel = name if rel_root == '.' else f"{rel_root}/{name}".replace('\\', '/') + if not (is_known_state_file(rel) or ignore.ignores(rel)): + continue + if os.path.lexists(new_dir / rel): + continue + keep.append(rel) + return keep + + +def carry_over_local_files( + old_dir: Path, new_dir: Path +) -> Tuple[List[str], List[Tuple[str, str]]]: + """Copy the plugin's local files from ``old_dir`` into ``new_dir``. + + Copies rather than moves, so ``old_dir`` stays a complete copy until the + caller deletes it. Returns ``(copied, failed)`` where ``failed`` pairs a + relative path with the error; the caller should keep ``old_dir`` when + anything failed. + """ + copied: List[str] = [] + failed: List[Tuple[str, str]] = [] + try: + candidates = local_files_to_keep(old_dir, new_dir) + except OSError as e: + return copied, [('.', str(e))] + for rel in candidates: + dest = Path(new_dir) / rel + try: + dest.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(Path(old_dir) / rel, dest) + copied.append(rel) + except OSError as e: + failed.append((rel, str(e))) + return copied, failed diff --git a/src/plugin_system/store_install.py b/src/plugin_system/store_install.py index 1665f7bc..8e8a90b4 100644 --- a/src/plugin_system/store_install.py +++ b/src/plugin_system/store_install.py @@ -22,6 +22,7 @@ from src.plugin_system.plugin_loader import ( contained_plugin_dir, requirements_to_install, ) from src.plugin_system.plugin_dirs import BACKUP_MARKER +from src.plugin_system.plugin_local_files import carry_over_local_files from src.plugin_system.repo_urls import ( USER_AGENT, github_api_headers, github_owner_repo, normalize_repo_url, ) @@ -92,7 +93,9 @@ class _InstallMixin: raise if installed: - self._discard_backup(plugin_id, backup_path, "install") + self._discard_backup( + plugin_id, backup_path, "install", + new_path=self._existing_install(plugin_id) or plugin_path) return True self._restore_backup(plugin_id, plugin_path, backup_path, "Install") @@ -133,8 +136,33 @@ class _InstallMixin: return f"could not set aside {plugin_path}: {e}" return None - def _discard_backup(self, plugin_id: str, backup_path: Path, action: str) -> None: - """Remove the set-aside copy after a successful (re)install.""" + def _discard_backup( + self, plugin_id: str, backup_path: Path, action: str, + new_path: Optional[Path] = None, + ) -> None: + """Remove the set-aside copy after a successful (re)install. + + With ``new_path`` (where the new copy landed), first carries the + plugin's own runtime files -- OAuth tokens, client secrets, anything + its .gitignore excludes -- from the old copy into the new one: no + release contains them, so deleting the old copy would destroy them. + See src/plugin_system/plugin_local_files.py. If any could not be + copied the old copy is kept, so nothing is lost. + """ + if new_path is not None and new_path.is_dir(): + copied, failed = carry_over_local_files(backup_path, new_path) + if copied: + self.logger.info( + "Kept %d local file(s) of %s across the %s: %s", + len(copied), plugin_id, action, ", ".join(copied)) + if failed: + self.logger.error( + "Could not carry %s's local files into the new copy (%s); " + "the previous copy is kept at %s -- copy them back by hand", + plugin_id, + "; ".join(f"{rel}: {err}" for rel, err in failed), + backup_path) + return if not self._safe_remove_directory(backup_path): self.logger.warning( "%s of %s succeeded but the previous copy at %s could not be " @@ -542,7 +570,8 @@ class _InstallMixin: raise temp_dir = None # Prevent cleanup since we moved it if backup_path is not None: - self._discard_backup(plugin_id, backup_path, "install") + self._discard_backup( + plugin_id, backup_path, "install", new_path=final_path) # Install dependencies self._install_dependencies(final_path) diff --git a/src/plugin_system/store_update.py b/src/plugin_system/store_update.py index 89f63a4b..4c1ec480 100644 --- a/src/plugin_system/store_update.py +++ b/src/plugin_system/store_update.py @@ -10,6 +10,9 @@ import subprocess # nosec B404 - list-form argv only, no shell # nosemgrep from pathlib import Path from typing import Dict, Optional, Tuple from src.plugin_system.plugin_dirs import BACKUP_MARKER +from src.plugin_system.plugin_local_files import ( + KNOWN_STATE_PATTERNS, is_known_state_file, +) from src.plugin_system.repo_urls import same_repo @@ -302,7 +305,11 @@ class _UpdateMixin: installed = False if installed: - self._discard_backup(plugin_id, backup_path, "update") + # install_plugin may land the new copy under the manifest id + # rather than the old directory name. + self._discard_backup( + plugin_id, backup_path, "update", + new_path=self._existing_install(plugin_id) or plugin_path) return True # Bad network, registry error...: the user keeps a working plugin. @@ -509,8 +516,12 @@ class _UpdateMixin: for line in untracked_result.stdout.strip().split('\n'): if line.startswith('??'): # Untracked file - file_path = line[3:].strip() - untracked_files.append(file_path) + file_path = line[3:].strip().strip('"') + # Tokens and secrets stay out of the + # stash (see below), so they alone are + # not a reason to stash. + if not is_known_state_file(file_path): + untracked_files.append(file_path) # Check for tracked file changes status_result = subprocess.run( @@ -537,9 +548,17 @@ class _UpdateMixin: if has_changes: self.logger.info(f"Stashing local changes in {plugin_id} before update") try: - # Use -u to include untracked files in stash + # Use -u to include untracked files in stash -- + # except the plugin's tokens and secrets, which a + # repo may have forgotten to gitignore. The stash + # is never popped, so a stashed token.pickle would + # vanish from the plugin and break it. + stash_cmd = ( + ['git', '-C', str(plugin_path), 'stash', 'push', '-u', + '-m', f'LEDMatrix auto-stash before update {plugin_id}', '--', '.'] + + [f':(exclude,glob)**/{p}' for p in KNOWN_STATE_PATTERNS]) stash_result = subprocess.run( - ['git', '-C', str(plugin_path), 'stash', 'push', '-u', '-m', f'LEDMatrix auto-stash before update {plugin_id}'], + stash_cmd, capture_output=True, text=True, timeout=30, diff --git a/test/test_store_update_keeps_local_files.py b/test/test_store_update_keeps_local_files.py new file mode 100644 index 00000000..28618236 --- /dev/null +++ b/test/test_store_update_keeps_local_files.py @@ -0,0 +1,244 @@ +"""A plugin update must keep the files the plugin wrote beside itself. + +Field incident, 2026-10-04: updating calendar 1.2.9 -> 1.2.12 from the web UI +replaced plugin-repos/calendar/ with the fresh download and deleted the old +copy -- and with it token.pickle and credentials.json, the plugin's Google +OAuth files. No release contains them (the repo gitignores them), so the hot +reload logged "Credentials file not found" and the calendar stayed broken +until the files were restored by hand. + +Both update routes are covered: a monorepo plugin (registry ``plugin_path``), +which is reinstalled into a fresh directory, and a plugin installed from its +own git repository, which is updated with ``git pull`` after an auto-stash. +""" + +import json +import shutil +import subprocess + +import pytest + +from src.plugin_system.plugin_local_files import ( + is_known_state_file, local_files_to_keep, +) +from src.plugin_system.store_manager import PluginStoreManager + +PLUGIN_ID = "calendar" + + +def _manifest(version): + return {"id": PLUGIN_ID, "name": "Calendar", "class_name": "CalendarPlugin", + "display_modes": ["calendar"], "version": version} + + +def _write_release(target, version): + """What a download of ``version`` puts on disk.""" + target.mkdir(parents=True, exist_ok=True) + (target / "manifest.json").write_text(json.dumps(_manifest(version))) + (target / "manager.py").write_text(f"VERSION = {version!r}\n") + (target / ".gitignore").write_text("credentials.json\ntoken.pickle\ncache/\n") + + +def _drop_local_files(plugin_dir): + """What the plugin writes at runtime: OAuth files plus cached state.""" + (plugin_dir / "token.pickle").write_bytes(b"\x80\x04oauth-token") + (plugin_dir / "credentials.json").write_text('{"installed": {}}') + (plugin_dir / "cache").mkdir() + (plugin_dir / "cache" / "events.json").write_text("[]") + + +def _assert_local_files_kept(plugin_dir): + assert (plugin_dir / "token.pickle").read_bytes() == b"\x80\x04oauth-token" + assert (plugin_dir / "credentials.json").read_text() == '{"installed": {}}' + assert (plugin_dir / "cache" / "events.json").read_text() == "[]" + + +def _leftover_backups(plugins_dir): + return [p.name for p in plugins_dir.iterdir() if "standalone-backup" in p.name] + + +@pytest.fixture +def store(tmp_path, monkeypatch): + mgr = PluginStoreManager( + plugins_dir=str(tmp_path / "plugin-repos"), + uninstalled_registry_path=str(tmp_path / "uninstalled.json")) + mgr.plugins_dir.mkdir(parents=True, exist_ok=True) + monkeypatch.setattr(mgr, "_install_dependencies", lambda *a, **k: True) + monkeypatch.setattr(mgr, "fetch_registry", lambda *a, **k: {"plugins": []}) + return mgr + + +class TestMonorepoUpdate: + @pytest.fixture + def installed(self, store, monkeypatch): + registry_entry = { + "id": PLUGIN_ID, "repo": "https://github.com/ChuckBuilds/ledmatrix-plugins", + "plugin_path": "plugins/calendar", "branch": "main", + "latest_version": "1.2.9", + } + monkeypatch.setattr(store, "get_plugin_info", lambda *a, **k: registry_entry) + release = {"version": "1.2.9"} + + def fake_monorepo_download(download_url, plugin_subpath, target): + assert plugin_subpath == "plugins/calendar" + _write_release(target, release["version"]) + return True + + monkeypatch.setattr(store, "_install_from_monorepo", fake_monorepo_download) + assert store.install_plugin(PLUGIN_ID) is True + + def publish(version): + registry_entry["latest_version"] = release["version"] = version + return store, store.plugins_dir / PLUGIN_ID, publish + + def test_update_keeps_token_and_gitignored_files(self, installed): + store, plugin_dir, publish = installed + _drop_local_files(plugin_dir) + + publish("1.2.12") + assert store.update_plugin(PLUGIN_ID) is True + + assert json.loads((plugin_dir / "manifest.json").read_text())["version"] == "1.2.12" + _assert_local_files_kept(plugin_dir) + assert _leftover_backups(store.plugins_dir) == [] + + def test_token_is_kept_even_when_the_release_does_not_gitignore_it(self, installed): + store, plugin_dir, publish = installed + (plugin_dir / ".gitignore").unlink() + (plugin_dir / "token.pickle").write_bytes(b"tok") + (plugin_dir / "config_secrets.json").write_text("{}") + + publish("1.2.12") + assert store.update_plugin(PLUGIN_ID) is True + + assert (plugin_dir / "token.pickle").read_bytes() == b"tok" + assert (plugin_dir / "config_secrets.json").read_text() == "{}" + + def test_release_content_wins_and_old_code_is_not_carried(self, installed): + store, plugin_dir, publish = installed + # A file the old copy had that the new release dropped, byte code, and + # an old copy of a file the new release also ships. + (plugin_dir / "removed_module.py").write_text("OLD = True\n") + (plugin_dir / "__pycache__").mkdir() + (plugin_dir / "__pycache__" / "manager.cpython-313.pyc").write_bytes(b"pyc") + + publish("1.2.12") + assert store.update_plugin(PLUGIN_ID) is True + + assert not (plugin_dir / "removed_module.py").exists() + assert not (plugin_dir / "__pycache__").exists() + assert "1.2.12" in (plugin_dir / "manager.py").read_text() + + def test_reinstall_over_an_existing_copy_keeps_them_too(self, installed): + store, plugin_dir, publish = installed + _drop_local_files(plugin_dir) + + assert store.install_plugin(PLUGIN_ID) is True + + _assert_local_files_kept(plugin_dir) + assert _leftover_backups(store.plugins_dir) == [] + + +class TestInstallFromUrlReplace: + def test_replacing_an_installed_copy_keeps_the_token(self, store, monkeypatch): + plugin_dir = store.plugins_dir / PLUGIN_ID + _write_release(plugin_dir, "1.0.0") + _drop_local_files(plugin_dir) + + def fake_clone(repo_url, target, branches): + _write_release(target, "2.0.0") + return "main" + + monkeypatch.setattr(store, "_install_via_git", fake_clone) + result = store.install_from_url( + "https://github.com/example/ledmatrix-calendar", plugin_id=PLUGIN_ID) + + assert result["success"] is True + assert json.loads((plugin_dir / "manifest.json").read_text())["version"] == "2.0.0" + _assert_local_files_kept(plugin_dir) + + +def _git(*args, cwd): + subprocess.run(["git", "-c", "user.email=t@example.com", "-c", "user.name=t", + "-c", "core.autocrlf=false", *args], + cwd=cwd, check=True, capture_output=True) + + +@pytest.mark.skipif(shutil.which("git") is None, reason="git not installed") +class TestGitRepoUpdate: + @pytest.fixture + def cloned(self, store, tmp_path, monkeypatch): + monkeypatch.setattr(store, "get_plugin_info", lambda *a, **k: None) + upstream = tmp_path / "upstream" + _write_release(upstream, "1.0.0") + # This repo does NOT gitignore the token: an untracked, non-ignored + # file is exactly what `git stash push -u` used to sweep away. + (upstream / ".gitignore").write_text("cache/\n") + _git("init", "-q", "-b", "main", cwd=upstream) + _git("add", ".", cwd=upstream) + _git("commit", "-qm", "1.0.0", cwd=upstream) + + plugin_dir = store.plugins_dir / PLUGIN_ID + _git("clone", "-q", str(upstream), str(plugin_dir), cwd=tmp_path) + + def publish(version): + (upstream / "manifest.json").write_text(json.dumps(_manifest(version))) + _git("commit", "-qam", version, cwd=upstream) + return store, plugin_dir, publish + + def test_pull_update_keeps_untracked_token(self, cloned): + store, plugin_dir, publish = cloned + _drop_local_files(plugin_dir) + # An unrelated untracked file, so the update really does stash. + (plugin_dir / "notes.txt").write_text("scratch") + + publish("1.1.0") + assert store.update_plugin(PLUGIN_ID) is True + + assert json.loads((plugin_dir / "manifest.json").read_text())["version"] == "1.1.0" + _assert_local_files_kept(plugin_dir) + + def test_token_alone_does_not_trigger_a_stash(self, cloned): + store, plugin_dir, publish = cloned + (plugin_dir / "token.pickle").write_bytes(b"tok") + + publish("1.1.0") + assert store.update_plugin(PLUGIN_ID) is True + + assert (plugin_dir / "token.pickle").read_bytes() == b"tok" + stashes = subprocess.run(["git", "-C", str(plugin_dir), "stash", "list"], + capture_output=True, text=True, check=True) + assert stashes.stdout.strip() == "" + + +class TestWhatIsKept: + @pytest.mark.parametrize("path,expected", [ + ("token.pickle", True), + ("data/session.pickle", True), + ("credentials.json", True), + ("token.json", True), + ("config_secrets.json", True), + (".pkce_code_verifier", True), + ("manager.py", False), + ("config.json", False), + ]) + def test_known_state_files(self, path, expected): + assert is_known_state_file(path) is expected + + def test_gitignore_rules(self, tmp_path): + old, new = tmp_path / "old", tmp_path / "new" + new.mkdir() + for rel in ["a.log", "logs/x.txt", "sub/deep/b.log", "keep.log", + "anchored.txt", "sub/anchored.txt", "assets/x/y_backup/z.png", + "manager.py", "shipped.log"]: + (old / rel).parent.mkdir(parents=True, exist_ok=True) + (old / rel).write_text("x") + (new / "shipped.log").write_text("new") + (old / ".gitignore").write_text( + "# comment\n*.log\n!keep.log\nlogs/\n/anchored.txt\n" + "assets/**/*_backup/\n") + + assert local_files_to_keep(old, new) == [ + "a.log", "anchored.txt", "assets/x/y_backup/z.png", + "logs/x.txt", "sub/deep/b.log", + ] From b638b91169ad4be515c488cf3062c6d09e9214d3 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Mon, 5 Oct 2026 09:36:12 -0400 Subject: [PATCH 2/8] feat(common): sports_game_over -- the reconciled game-over check (sports family 5) (#770) * feat(common): sports_game_over -- the reconciled game-over check (sports family 5) New hardware-free module src/common/sports_game_over.py with SportsGameOverMixin._is_game_really_over, the scoreboards' SportsLive check that drops a game ESPN still lists as live, copied from ledmatrix-plugins claude/family5-reconcile once the nine copies (five bodies) became one. Over on a final period text; from period FINAL_PERIOD on, also on a 0:00 clock string unless the score is level (a tie at the end of regulation goes to overtime; a game that ends tied ends on its final status). FINAL_PERIOD is the one per-sport seam, a class attribute defaulting to None (the clock never ends a game); the scoreboards declare 3 (hockey), 4 (basketball, football, lacrosse) or None (afl, nrl, soccer, baseball, ufc). - test/test_sports_game_over.py: the plugins' pinned matrix folded to the three FINAL_PERIOD values, edge shapes, the tie guard, ufc's recorded ESPN MMA states, an override deferring through super() (baseball), the base order with SportsLiveSharedMixin._detect_stale_games, host contract. - test/test_sports_game_over_parity.py: with LEDMATRIX_PLUGINS, compares the body with every plugin copy (drift-report normalisation plus decorators) and each plugin's FINAL_PERIOD with the owner's decision. - mypy ratchet, src/common/README.md, CHANGELOG (Unreleased, New modules). - docs/SPORTS_UNIFICATION.md: family 5 status and decisions; the seam tables now match the code (FINAL_PERIOD defaults to None; the CLOCK_COUNTS_DOWN seam never existed and is gone from the doc). Co-Authored-By: Claude Opus 5.5 * docs: cite ledmatrix-plugins #625 for the family 5 reconcile Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 10 + docs/SPORTS_UNIFICATION.md | 55 ++++-- mypy-clean.txt | 1 + src/common/README.md | 11 ++ src/common/sports_game_over.py | 125 ++++++++++++ src/common/sports_shared.py | 4 +- test/test_sports_game_over.py | 275 +++++++++++++++++++++++++++ test/test_sports_game_over_parity.py | 123 ++++++++++++ 8 files changed, 584 insertions(+), 20 deletions(-) create mode 100644 src/common/sports_game_over.py create mode 100644 test/test_sports_game_over.py create mode 100644 test/test_sports_game_over_parity.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ba95dce..0a260dad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -668,6 +668,16 @@ policies are unchanged. - `src/display_arbiter.py` -- the display loop's Arbiter (see Tooling). Core-internal: plugins have no reason to import it, so it sets no `ledmatrix_min_version` floor. +- `src/common/sports_game_over.py` -- `SportsGameOverMixin`, sports + consolidation family 5: `_is_game_really_over`, the scoreboards' + `SportsLive` check that drops a game ESPN still lists as live, once the + plugins made their five bodies one. Over on a final period text, or on a + 0:00 clock from period `FINAL_PERIOD` on unless the score is level (a tie + at the end of regulation goes to overtime). `FINAL_PERIOD` is the per-sport + class attribute, `None` by default (the clock never ends a game); the + scoreboards declare 3 (hockey), 4 (basketball, football, lacrosse) or + `None`. List the mixin before `SportsLiveSharedMixin`. A plugin may import + it once it floors on the release that ships it, and deletes its copy then. ### Tooling diff --git a/docs/SPORTS_UNIFICATION.md b/docs/SPORTS_UNIFICATION.md index fe72fe06..3c7072f2 100644 --- a/docs/SPORTS_UNIFICATION.md +++ b/docs/SPORTS_UNIFICATION.md @@ -91,6 +91,7 @@ more. Shared sports code lives in `src/common`: | `sports_live_scroll.py` | next release | `SportsLiveScrollMixin` — rebuild a live scroll strip mid-cycle, keeping the marquee's place | | `sports_display_rules.py` | next release | `SportsCardOptionsMixin`, `SportsGameRulesMixin` — scorebug date options, the no-favourites filter, non-favourite live dwell | | `sports_font_path.py` | next release | `resolve_font_path` — what the plugins' `_resolve_font_path` copies return | +| `sports_game_over.py` | next release | `SportsGameOverMixin` — `_is_game_really_over`, with the `FINAL_PERIOD` seam (family 5) | Each is described in [src/common/README.md](../src/common/README.md). @@ -134,8 +135,7 @@ constants rather than behavior: | Attribute | Meaning | Default | |---|---|---| -| `FINAL_PERIOD` | Period at/after which a zero clock can mean "over" | `4` (hockey overrides to `3`) | -| `CLOCK_COUNTS_DOWN` | Whether `0:00` means "expired" | `True` (soccer/afl/nrl override to `False` — their clocks count up, so `0:00` is kickoff) | +| `FINAL_PERIOD` | Period from which a 0:00 clock ends a game (`sports_game_over`) | `None`: the clock never ends a game (afl, nrl, soccer, baseball, ufc). Hockey sets `3`; basketball, football and lacrosse `4` | | `COALESCE_SCORING_SEQUENCE` | Fold score increments arriving during an active celebration into that one celebration | `False` (football overrides to `True` — a touchdown lands as +6, then +1 for the extra point) | ### Why these are seams and not branches @@ -146,11 +146,14 @@ so NRL matches favorites on team ID. Flattening every plugin to abbreviations would silently select the wrong club for NRL users. The base declares the seam, NRL fills it, and core never learns the string `"nrl"`. -`CLOCK_COUNTS_DOWN` exists for the same reason in the opposite direction: a +`FINAL_PERIOD` exists for the same reason in the opposite direction: a soccer clock reading `0:00` means the match has not kicked off, so running the -clock-expiry branch there would evict live games. +clock-expiry rule there would evict live games. Those sports declare `None`, +and so do baseball (innings, not a clock) and ufc (a bout ends only on ESPN's +final status). One attribute covers both questions, whether the clock can end +a game and from which period, so no separate count-down flag was added. -`COALESCE_SCORING_SEQUENCE` is the third of the same kind. In football one +`COALESCE_SCORING_SEQUENCE` is another of the same kind. In football one scoring play arrives as two score updates, so the follow-up must be folded into the first celebration; in soccer two increments a few seconds apart are two real goals, and folding them would swallow one. Neither default is "right" — which is @@ -298,6 +301,20 @@ Left in the plugins, though identical: renderers) is already core's, in `SportsHelpersMixin`; a renderer that wants it can inherit that. +### Family 5: the game-over check (core done; adoption waits for a release) + +The pilot of the method below. ledmatrix-plugins `scripts/test_game_over_check.py` +(#621) pinned 3,115 answers across the nine plugins first; the reconcile +(ledmatrix-plugins #625) made the five bodies one and +changed only the cells the owner's decisions under +[Product decisions](#product-decisions-each-family-needs) explain: ufc's +clock rule (65 cells), baseball's dormant one (53, every one a game with a +`period` baseball's games never carry), and a level score at 0:00 (five +cells in hockey, basketball, football and lacrosse). The harness renders +were pixel-identical. `src/common/sports_game_over.py` holds the body; +`test/test_sports_game_over_parity.py` compares it, and each plugin's +`FINAL_PERIOD`, with the plugin copies. + ### Why the method changes Byte-identical promotion has nearly run dry. Measured on ledmatrix-plugins @@ -333,8 +350,8 @@ game-over check); the report measures each method in it. The procedure: line in each plugin. - *A per-sport fact* (hockey ends in period 3; a soccer clock counts up). Make it a declared class constant or override point with a default, as - `FINAL_PERIOD`, `CLOCK_COUNTS_DOWN`, `COALESCE_SCORING_SEQUENCE` and - `_favorite_key` are, and add it to the tables above. Never a sport-name + `FINAL_PERIOD`, `COALESCE_SCORING_SEQUENCE` and `_favorite_key` are, + and add it to the tables above. Never a sport-name branch: core must not learn sport names. - *A product difference*: anything a user can see (which games show, a colour, a date, a badge, how long a screen stays). The owner picks the @@ -382,7 +399,7 @@ release. | # | Family | Methods (variants) | Why here | |---|---|---|---| | 4 | Identical sweep | `manager.py`: `_dispatch_switch_refresh`, `_favorite_team_is_live`, `get_vegas_priority_weight`, `_game_involves`, `_favorite_scan_targets`, `_favorite_scan_games`, `_get_total_games_for_manager` (all nine, 1); the live-scroll helpers `_preserving_scroll_position`, `_refresh_live_scroll_managers`, `_live_scroll_managers`, `_note_live_scroll_built`, `_live_scroll_needs_rebuild`, `_live_scroll_fields` (eight, 1). `sports.py`: `_card_option`, `_filtered_or_all`, `_effective_live_duration`, `_recent_date_text` (eight, 1). 58 identical families in all | Nothing to decide; brings `manager.py` into core as a `SportsPluginHostMixin`. `_resolve_font_path` (identical in nine `sports.py` and eight renderers) becomes `sports_font_path.resolve_font_path`, not `font_layout.resolve_asset_path`, which skips the cwd. Core side done; see [Stage 4](#stage-4-the-identical-sweep-core-done-adoption-waits-for-a-release) | -| 5 | Game-over check | `SportsLive._is_game_really_over` (5) | Pure logic, no pixels; its seams (`FINAL_PERIOD`, `CLOCK_COUNTS_DOWN`) were designed in B1. The pilot for the procedure | +| 5 | Game-over check | `SportsLive._is_game_really_over` (5) | Pure logic, no pixels; one seam, `FINAL_PERIOD`. The pilot for the procedure. Reconciled to one body and promoted as `sports_game_over`; adoption waits for the release that ships it. See [Family 5](#family-5-the-game-over-check-core-done-adoption-waits-for-a-release) | | 6 | Favourite matching | `_is_favorite_game` (7 across three classes), `_select_games_for_display` (2: nrl), `_select_recent_games_for_display` (3) | Everything that asks "is this a favourite" goes through the 3.5.0 `_favorite_key` seam | | 7 | Other-games rotation | `_by_importance`, `_other_games_window`, `_advance_other_games_if_due` (2 each: football), `_rotate_other_games_on_display` (2: ufc) | One outlier each; football carries two fixes the other eight lack | | 8 | Rankings | `_fetch_team_rankings` (3), `_choose_poll` (3), `_load_division_team_ids`, `_passes_other_filters`, `_best_rank`, `_is_ranked_game` (2 each: football) | Needs 7; the rank badge and the "ranked only" filter read it | @@ -416,17 +433,17 @@ family 9 prepares. Owner calls to make before (or while) reconciling. Items marked *verify* are suspected behaviour that needs a payload or a rig to confirm first. -- **5, game-over check.** Which rule each sport gets: the clock never ends a - game in afl, nrl and soccer (`CLOCK_COUNTS_DOWN = False`); hockey ends at - 0:00 from period 3, basketball, football and lacrosse from period 4. - baseball and ufc share a copy that reads a missing clock as "0:00": dormant - in baseball (its games carry no `period`), and not triggered by ufc's round - breaks either. ESPN sends a break as `STATUS_END_OF_ROUND` with displayClock - `-`, not `0:00` (verified against recorded payloads; ledmatrix-plugins#580 - pins it). Whatever rule ufc gets must not read `-` as `0:00`. Decide ufc's - rule: no clock rule (ESPN's `STATUS_FINAL` is the only end signal it needs; - this also closes a ~1 s window at the horn when the ticking clock reads - `0:00`), or its own final period. +- **5, game-over check. Decided 2026-10-05, done:** one seam, + `FINAL_PERIOD`: hockey 3; basketball, football and lacrosse 4; `None` (the + clock never ends a game) for afl, nrl and soccer (clocks that count up), + baseball (its games carry no `period`, so the old rule was dormant) and + ufc (a bout ends only on ESPN's final status, which also closes the ~1 s + window at the horn when the ticking clock reads `0:00`; ESPN's round-break + displayClock `-` was never a zero clock, ledmatrix-plugins#580). Only a + non-empty clock string counts (the baseball/ufc copy read a missing clock + as `0:00`). A score level at 0:00 is not over: the game stays live through + the break before overtime, and one that really ends tied ends on its final + status. Baseball keeps its postponed/suspended override in `BaseballLive`. - **6, favourite matching.** NRL keeps matching favourites by team id (abbreviations collide: NEW, CAN), through `_favorite_key` rather than its own copies of the selection methods. Six plugins log the recent-games diff --git a/mypy-clean.txt b/mypy-clean.txt index 63f6f5bc..ffc7a6c5 100644 --- a/mypy-clean.txt +++ b/mypy-clean.txt @@ -38,6 +38,7 @@ src/common/sports_celebration.py src/common/sports_display_rules.py src/common/sports_fetch.py src/common/sports_font_path.py +src/common/sports_game_over.py src/common/sports_live_scroll.py src/common/sports_plugin_host.py src/common/sports_scroll.py diff --git a/src/common/README.md b/src/common/README.md index a0f46287..b84198c7 100644 --- a/src/common/README.md +++ b/src/common/README.md @@ -46,6 +46,7 @@ Rules for the package: | [`sports_display_rules`](#sports_display_rules) | Which games a scoreboard shows, for how long, and its scorebug date line | Yes (scoreboards) | 3.8.0 | | [`sports_fetch`](#sports_fetch) | Scoreboard season fetch, lookback and live-odds decisions | Yes (scoreboards) | 3.7.0 | | [`sports_font_path`](#sports_font_path) | Find a scoreboard's bundled font whatever the cwd | Yes (scoreboards) | 3.8.0 | +| [`sports_game_over`](#sports_game_over) | Whether a game ESPN still lists as live has ended | Yes (scoreboards) | next release | | [`sports_game_renderer`](#sports_game_renderer) | Scoreboard scroll/Vegas card geometry | Yes (scoreboards) | 3.3.0 | | [`sports_helpers`](#sports_helpers) | Small helpers every scoreboard `sports.py` copies | Yes (scoreboards) | 3.5.0 | | [`sports_live_scroll`](#sports_live_scroll) | Rebuild a live scroll strip mid-cycle without moving it | Yes (scoreboards) | 3.8.0 | @@ -294,6 +295,16 @@ path as given when it exists (relative to the cwd), else `font_layout.resolve_asset_path(path)`. What the scoreboards' `_resolve_font_path` copies return on a core that ships it. +### sports_game_over + +[`sports_game_over.py`](sports_game_over.py). `SportsGameOverMixin`: +`_is_game_really_over(game)`, the `SportsLive` check that drops a game ESPN +still lists as live (`SportsLiveSharedMixin._detect_stale_games` calls it). +Over on a final period text, or on a 0:00 clock from period `FINAL_PERIOD` +on unless the score is level. `FINAL_PERIOD` is a class attribute the host +sets per sport; the default `None` means the clock never ends a game. List +it before `SportsLiveSharedMixin`. + ### sports_game_renderer [`sports_game_renderer.py`](sports_game_renderer.py). diff --git a/src/common/sports_game_over.py b/src/common/sports_game_over.py new file mode 100644 index 00000000..3288af7e --- /dev/null +++ b/src/common/sports_game_over.py @@ -0,0 +1,125 @@ +"""Whether a game ESPN still lists as live has in fact ended (sports family 5). + +``SportsGameOverMixin._is_game_really_over`` is the scoreboards' +``SportsLive._is_game_really_over``, reconciled in ledmatrix-plugins +#625 from five bodies into one and copied here under +its existing name. ``SportsLiveSharedMixin._detect_stale_games`` +(``src.common.sports_shared``) calls it on every live game, and the plugins' +live-priority filters call it too, to drop a game ESPN still reports as +in progress. + +A game is over when its period text says final. From period ``FINAL_PERIOD`` +on, a clock reading 0:00 ends it too, unless the score is level: a tie at the +end of regulation goes to overtime (or a shootout), and a game that does end +tied says final. Only a clock *string* is read ("0:00" and ":00" are zero; +":40", "0.0" and ESPN's "-" between MMA rounds are not), and a missing or +unreadable score leaves the decision to the clock. + +``FINAL_PERIOD`` is the one per-sport fact, a class attribute rather than a +sport-name branch. The scoreboards declare it on their ``SportsLive``: + +- 3: hockey; +- 4: basketball, football, lacrosse; +- ``None`` (this default; the clock never ends a game): afl, nrl and soccer, + whose clocks count up; baseball, which has innings; ufc, whose bouts end + only on ESPN's final status. + +A sport can still override the method and defer to it, as baseball's +``BaseballLive`` does to end postponed and suspended games first. + +A new module rather than another method on ``sports_shared``, for the reason +``sports_helpers`` gives: a missing module fails at load, where the version +checks see it; a missing method fails mid-update. + +WHAT A HOST MUST PROVIDE +------------------------ +Derived by walking every ``self.`` the mixin reads; the host-contract +test in ``test/test_sports_game_over.py`` fails if a read is added without +being listed here. + +- ``logger`` -- a ``logging.Logger``; the method logs its verdict at DEBUG. +- ``FINAL_PERIOD`` -- defaulted here to ``None``; set it on the host class. + +The method reads the game dict's ``away_abbr``, ``home_abbr``, +``period_text``, ``period``, ``clock``, ``away_score`` and ``home_score`` +(``_extract_game_details_common``'s keys); any of them may be missing or +null. + +BASE ORDER +---------- +List the mixin before ``SportsLiveSharedMixin`` -- +``class SportsLive(SportsGameOverMixin, SportsLiveSharedMixin, SportsCore)`` -- +so the shared mixin's ``_detect_stale_games`` finds this method through the +MRO. Neither shared mixin defines it, so the order does not change which body +runs today; it keeps the method next to its caller should one ever be added +there. A method on the plugin's own class still wins, and its ``super()`` +reaches this one. The mixin has no ``__init__`` and no state. +""" + +import logging +from typing import Dict, Optional + + +class SportsGameOverMixin: + """The live manager's "is this game really over?" check. See module docstring.""" + + # The host contract, declared for type checking only. + logger: logging.Logger + + #: Period from which a 0:00 clock ends a game; None: the clock never does. + FINAL_PERIOD: Optional[int] = None + + def _is_game_really_over(self, game: Dict) -> bool: + """Whether a game ESPN still lists as live has in fact ended. + + It has when its period text says final. From period ``FINAL_PERIOD`` + on, a clock at 0:00 ends it too, unless the score is level: a tie at + the end of regulation goes to overtime, and a game that does end tied + says final. With ``FINAL_PERIOD = None`` the clock never ends a game. + """ + game_str = f"{game.get('away_abbr')}@{game.get('home_abbr')}" + + # ESPN can send the key as null, and .get()'s default only covers a + # missing key, so a None here crashed the whole live update. + raw_period_text = game.get("period_text") + period_text = raw_period_text.lower() if isinstance(raw_period_text, str) else "" + if "final" in period_text: + self.logger.debug( + f"_is_game_really_over({game_str}): " + f"returning True - 'final' in period_text='{period_text}'" + ) + return True + + # Same for a null or non-numeric period: treat it as period 0. + try: + period = int(game.get("period") or 0) + except (TypeError, ValueError, OverflowError): + period = 0 + # Only a clock string is read: "0:00" and ":00" are zero; ":40" is not. + clock = game.get("clock") + clock_at_zero = isinstance(clock, str) and clock.replace(":", "").strip() in ("000", "00") + + if self.FINAL_PERIOD is not None and period >= self.FINAL_PERIOD and clock_at_zero: + try: + tied = int(game["away_score"]) == int(game["home_score"]) + except (KeyError, TypeError, ValueError, OverflowError): + tied = False # a missing or unreadable score leaves it to the clock + if not tied: + self.logger.debug( + f"_is_game_really_over({game_str}): " + f"returning True - clock at 0:00 (clock='{clock}', period={period})" + ) + return True + self.logger.debug( + f"_is_game_really_over({game_str}): " + f"returning False - tied at 0:00 (period={period}), overtime next" + ) + return False + + self.logger.debug( + f"_is_game_really_over({game_str}): returning False" + ) + return False + + +__all__ = ["SportsGameOverMixin"] diff --git a/src/common/sports_shared.py b/src/common/sports_shared.py index 954a9b1f..f1efbbd7 100644 --- a/src/common/sports_shared.py +++ b/src/common/sports_shared.py @@ -57,7 +57,9 @@ Methods that stay per-plugin, because they are not identical across the eight ``_get_layout_offset``, ``_by_importance``, ``_other_games_window``, ``_upcoming_date_and_time_text``, ``_extract_game_details_common``, ``_load_division_team_ids``, ``_get_timezone``, ``_is_favorite_game``, -``_is_game_really_over``, ``_is_ranked_game``, ``_passes_other_filters``. +``_is_ranked_game``, ``_passes_other_filters``. (``_is_game_really_over``, +which ``_detect_stale_games`` below calls, was here too until the plugins +reconciled it; it is now ``src.common.sports_game_over``.) Of the fourteen shared class constants, thirteen are identical everywhere and live here. Only ``_SCORE_PROBE_TEXT`` varies -- afl and basketball reach three digits diff --git a/test/test_sports_game_over.py b/test/test_sports_game_over.py new file mode 100644 index 00000000..4e83e75f --- /dev/null +++ b/test/test_sports_game_over.py @@ -0,0 +1,275 @@ +"""src.common.sports_game_over: behaviour, host contract and base order. + +The matrix is ledmatrix-plugins' ``scripts/test_game_over_check.py`` (the +table the family 5 reconcile was checked against) folded to the three +``FINAL_PERIOD`` values the nine scoreboards declare: None (afl, baseball, +nrl, soccer, ufc), 4 (basketball, football, lacrosse) and 3 (hockey). +Baseball's postponed/suspended override stays in its plugin and is not here. +""" + +import ast +import logging +import time +from pathlib import Path + +import pytest + +from src.common import sports_game_over +from src.common.sports_game_over import SportsGameOverMixin +from src.common.sports_shared import SportsLiveSharedMixin + +LOG = logging.getLogger("test_sports_game_over") + + +def host(final_period): + """A live manager stand-in declaring ``FINAL_PERIOD`` as a plugin does.""" + cls = type("Live", (SportsGameOverMixin,), {"FINAL_PERIOD": final_period}) + h = cls() + h.logger = LOG + return h + + +MISSING = object() # the key is absent from the game dict + + +def game(period_text="", period=MISSING, clock=MISSING, away="1", home="2"): + g = {"away_abbr": "AWY", "home_abbr": "HOM", "away_score": away, + "home_score": home, "period_text": period_text} + if period is not MISSING: + g["period"] = period + if clock is not MISSING: + g["clock"] = clock + return g + + +# --------------------------------------------------------------------------- +# The matrix: clock x period, for each FINAL_PERIOD. Scores 1-2. +# --------------------------------------------------------------------------- + +FINAL_PERIODS = (None, 4, 3) +PERIODS = (MISSING, 1, 2, 3, 4, 5, 6) +CLOCKS = {"12:00": "12:00", "0:00": "0:00", ":00": ":00", "0.0": "0.0", + "-": "-", "''": "", "None": None, "missing": MISSING} + +#: The period text each ESPN status carries. Only "Final" contains "final"; +#: the method reads no status, so every other text answers the same row. +LIVE_TEXTS = { + "in progress": lambda p: "" if p is MISSING else f"P{p}", + "end of period": lambda p: "" if p is MISSING else f"End P{p}", + "halftime": lambda p: "Halftime", + "end of round": lambda p: "" if p is MISSING else f"End R{p}", + "postponed": lambda p: "Postponed", +} + +#: clock -> one cell per period (missing, 1..6) for FINAL_PERIOD None, 4, 3. +EXPECTED_LIVE = { + "12:00": "....... ....... .......", + "0:00": "....... ....YYY ...YYYY", + ":00": "....... ....YYY ...YYYY", + "0.0": "....... ....... .......", + "-": "....... ....... .......", + "''": "....... ....... .......", + "None": "....... ....... .......", + "missing": "....... ....... .......", +} + + +def row(text_for, clock): + return " ".join( + "".join("Y" if host(fp)._is_game_really_over(game(text_for(p), p, clock)) else "." + for p in PERIODS) + for fp in FINAL_PERIODS) + + +@pytest.mark.parametrize("status", sorted(LIVE_TEXTS)) +@pytest.mark.parametrize("clock_label", sorted(CLOCKS)) +def test_a_live_period_text(status, clock_label): + assert row(LIVE_TEXTS[status], CLOCKS[clock_label]) == EXPECTED_LIVE[clock_label] + + +@pytest.mark.parametrize("clock_label", sorted(CLOCKS)) +def test_a_final_period_text_is_always_over(clock_label): + assert row(lambda p: "Final", CLOCKS[clock_label]) == "YYYYYYY YYYYYYY YYYYYYY" + + +#: label -> (game, one cell per FINAL_PERIOD None, 4, 3) +EDGES = { + "period_text None, P4 0:00": (game(None, 4, "0:00"), ".YY"), + "period None, 0:00": (game("", None, "0:00"), "..."), + "period 'OT', 0:00": (game("OT", "OT", "0:00"), "..."), + "period '4' (str), 0:00": (game("P4", "4", "0:00"), ".YY"), + "clock int 0, P4": (game("P4", 4, 0), "..."), + "clock float 0.0, P4": (game("P4", 4, 0.0), "..."), + "clock ' 0:00 ', P4": (game("P4", 4, " 0:00 "), ".YY"), + "clock '00:00', P4": (game("P4", 4, "00:00"), "..."), + "period_text 'Final/OT', P5 0:00": (game("Final/OT", 5, "0:00"), "YYY"), + "period_text 'FINAL', P1 12:00": (game("FINAL", 1, "12:00"), "YYY"), +} + + +@pytest.mark.parametrize("label", sorted(EDGES)) +def test_edge_shapes(label): + g, want = EDGES[label] + got = "".join("Y" if host(fp)._is_game_really_over(dict(g)) else "." for fp in FINAL_PERIODS) + assert got == want + + +# --------------------------------------------------------------------------- +# The tie guard: level at 0:00 is overtime, not the end. +# --------------------------------------------------------------------------- + +class TestTieGuard: + @pytest.mark.parametrize("fp,period", [(4, 4), (4, 5), (3, 3), (3, 4), (3, 5)]) + def test_level_at_zero_is_not_over(self, fp, period): + assert host(fp)._is_game_really_over(game("", period, "0:00", "2", "2")) is False + + def test_level_scores_compare_as_numbers(self): + assert host(4)._is_game_really_over(game("", 4, "0:00", 2, "2")) is False + assert host(4)._is_game_really_over(game("", 4, "0:00", " 2 ", "2")) is False + + def test_a_game_that_ends_level_ends_on_final(self): + assert host(4)._is_game_really_over(game("Final/OT", 5, "0:00", "2", "2")) is True + + def test_level_before_the_final_period_was_never_over(self): + assert host(4)._is_game_really_over(game("", 3, "0:00", "2", "2")) is False + + @pytest.mark.parametrize("away,home", [ + (MISSING, MISSING), (None, None), ("", ""), ("2", None), + ("2.0", "2.0"), ({"value": 2}, {"value": 2}), ("inf", "inf"), + ]) + def test_an_unreadable_score_leaves_it_to_the_clock(self, away, home): + g = game("", 4, "0:00") + for key, value in (("away_score", away), ("home_score", home)): + if value is MISSING: + del g[key] + else: + g[key] = value + assert host(4)._is_game_really_over(g) is True + + def test_float_infinity_does_not_raise(self): + assert host(4)._is_game_really_over( + game("", 4, "0:00", float("inf"), float("inf"))) is True + + +# --------------------------------------------------------------------------- +# ufc: ESPN MMA payloads, as ufc's _extract_game_details stores them +# (ledmatrix-plugins plugins/ufc-scoreboard/test/fixtures/espn_mma_round_states.json). +# --------------------------------------------------------------------------- + +UFC_RECORDED = { + "in_round_3_of_3": ("R3", 3, "1:21"), + "break_after_round_1": ("R1", 1, "-"), + "end_of_round_after_stoppage": ("R2", 2, "0:51"), + "walkouts_five_rounder": ("", 0, "-"), + "final_five_round_decision": ("R5", 5, "5:00"), + "final_five_round_stoppage": ("R5", 5, "1:38"), + "final_three_round_decision": ("R3", 3, "5:00"), + "final_three_round_stoppage": ("R2", 2, "4:07"), + "break_after_round_4_of_5": ("R4", 4, "-"), + "end_of_round_5_awaiting_decision": ("R5", 5, "-"), +} + + +class TestUfc: + @pytest.mark.parametrize("name", sorted(UFC_RECORDED)) + def test_no_recorded_state_is_over_here(self, name): + """A finished bout leaves the live list on is_final, before this is asked.""" + text, period, clock = UFC_RECORDED[name] + assert host(None)._is_game_really_over(game(text, period, clock, "0", "0")) is False + + @pytest.mark.parametrize("fp", FINAL_PERIODS) + def test_a_round_break_dash_is_never_a_zero_clock(self, fp): + assert host(fp)._is_game_really_over(game("R4", 4, "-", "1", "2")) is False + + @pytest.mark.parametrize("clock", ["0:00", None, MISSING]) + def test_the_horn_does_not_end_a_bout(self, clock): + assert host(None)._is_game_really_over(game("R5", 5, clock, "1", "2")) is False + + +# --------------------------------------------------------------------------- +# Wiring: the default, overrides, and the live mixin's caller. +# --------------------------------------------------------------------------- + +def test_the_default_is_no_clock_rule(): + assert SportsGameOverMixin.FINAL_PERIOD is None + + +def test_an_override_defers_through_super(): + """baseball's BaseballLive: its own check first, then the shared one.""" + + class Baseballish(SportsGameOverMixin): + logger = LOG + + def _is_game_really_over(self, game): + if game.get("status") == "status_postponed": + return True + return super()._is_game_really_over(game) + + b = Baseballish() + assert b._is_game_really_over(dict(game("", 6, "0:00"), status="status_postponed")) is True + assert b._is_game_really_over(game("", 6, "0:00")) is False + assert b._is_game_really_over(game("Final", 9, None)) is True + + +class _Live(SportsGameOverMixin, SportsLiveSharedMixin): + """A SportsLive stand-in in the documented base order.""" + + FINAL_PERIOD = 4 + + def __init__(self): + self.logger = LOG + self.stale_game_timeout = 600 + self.game_update_timestamps = {} + + +class TestBaseOrder: + def test_the_documented_order_resolves_this_method(self): + assert _Live._is_game_really_over is SportsGameOverMixin._is_game_really_over + mro = _Live.__mro__ + assert mro.index(SportsGameOverMixin) < mro.index(SportsLiveSharedMixin) + + def test_neither_shared_mixin_defines_it(self): + """So the base order cannot change which body runs.""" + from src.common.sports_shared import SportsCoreSharedMixin + for mixin in (SportsLiveSharedMixin, SportsCoreSharedMixin): + assert "_is_game_really_over" not in vars(mixin) + + def test_detect_stale_games_drops_an_over_game_through_it(self): + live = _Live() + live.game_update_timestamps = {"over": {"last_seen": time.time()}, + "on": {"last_seen": time.time()}} + games = [dict(game("", 4, "0:00"), id="over"), + dict(game("", 4, "0:00", "2", "2"), id="on")] + live._detect_stale_games(games) + assert [g["id"] for g in games] == ["on"] + assert "over" not in live.game_update_timestamps + + def test_the_class_value_wins_over_the_default(self): + assert _Live().FINAL_PERIOD == 4 + assert _Live()._is_game_really_over(game("", 4, "0:00")) is True + + +# --------------------------------------------------------------------------- +# Host contract +# --------------------------------------------------------------------------- + +def _self_reads(): + tree = ast.parse(Path(sports_game_over.__file__).read_text(encoding="utf-8")) + cls = next(n for n in tree.body + if isinstance(n, ast.ClassDef) and n.name == "SportsGameOverMixin") + return {node.attr for node in ast.walk(cls) + if isinstance(node, ast.Attribute) and isinstance(node.ctx, ast.Load) + and isinstance(node.value, ast.Name) and node.value.id == "self"} + + +class TestHostContract: + def test_every_host_read_is_documented(self): + undocumented = sorted(n for n in _self_reads() + if f"``{n}``" not in sports_game_over.__doc__) + assert undocumented == [], f"read but not in the host contract: {undocumented}" + + def test_the_mixin_creates_no_state(self): + assert "__init__" not in vars(SportsGameOverMixin) + assert not hasattr(SportsGameOverMixin, "logger") + assert sorted(n for n in vars(SportsGameOverMixin) if not n.startswith("__")) == [ + "FINAL_PERIOD", "_is_game_really_over"] diff --git a/test/test_sports_game_over_parity.py b/test/test_sports_game_over_parity.py new file mode 100644 index 00000000..8f09aaee --- /dev/null +++ b/test/test_sports_game_over_parity.py @@ -0,0 +1,123 @@ +"""sports_game_over still matches every plugin copy, and each plugin's FINAL_PERIOD. + +``SportsGameOverMixin._is_game_really_over`` was copied from the scoreboards' +``SportsLive._is_game_really_over`` once family 5 had made the nine copies one +body. The plugins delete their copies once they floor on the release that +ships this module. Until each has, a copy that changes on its own is a fix one +side has and the other lacks. + +Point LEDMATRIX_PLUGINS at a ledmatrix-plugins checkout and the method is +compared with every plugin copy using ``scripts/sports_drift_report.py``'s own +normalisation (the AST with docstrings and annotations dropped), plus the +decorators. A copy that is gone counts as adopted when the plugin's +``sports.py`` names the module. Each plugin's ``SportsLive.FINAL_PERIOD`` is +compared with the value the owner decided for its sport, which stays in the +plugin after adoption. Without the variable this skips: core CI has no plugins +checkout. +""" + +import ast +import importlib.util +import os +from pathlib import Path + +import pytest + +from src.common import sports_game_over + +REPO = Path(__file__).resolve().parents[1] + +#: The owner's decision (docs/SPORTS_UNIFICATION.md, family 5): the period +#: from which a 0:00 clock ends a game, None where the clock never does. +FINAL_PERIOD = { + "afl": None, "baseball": None, "basketball": 4, "football": 4, + "hockey": 3, "lacrosse": 4, "nrl": None, "soccer": None, "ufc": None, +} +NAME = "_is_game_really_over" + + +def _drift_report(): + """scripts/sports_drift_report.py, loaded by path (scripts/ is no package).""" + spec = importlib.util.spec_from_file_location( + "sports_drift_report", REPO / "scripts" / "sports_drift_report.py") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +DRIFT = _drift_report() + + +def _plugins_root(): + root = DRIFT.resolve_plugins_dir(os.environ.get("LEDMATRIX_PLUGINS")) + if root is None: + pytest.skip("set LEDMATRIX_PLUGINS to a ledmatrix-plugins checkout to " + "compare this module against the plugin copies") + return root + + +def _class(tree, name): + return next(n for n in tree.body if isinstance(n, ast.ClassDef) and n.name == name) + + +def _method(cls): + return next((n for n in cls.body + if isinstance(n, ast.FunctionDef) and n.name == NAME), None) + + +def _fingerprint(node): + return (DRIFT._digest(node, DRIFT._Canonical()), + tuple(ast.unparse(d) for d in node.decorator_list)) + + +def _final_period(cls): + for node in cls.body: + if (isinstance(node, (ast.Assign, ast.AnnAssign)) and node.value is not None): + target = node.targets[0] if isinstance(node, ast.Assign) else node.target + if isinstance(target, ast.Name) and target.id == "FINAL_PERIOD": + return ast.literal_eval(node.value) + raise AssertionError("SportsLive declares no FINAL_PERIOD") + + +def _ours(): + tree = ast.parse(Path(sports_game_over.__file__).read_text(encoding="utf-8")) + return _class(tree, "SportsGameOverMixin") + + +def test_the_mixin_holds_one_method_and_the_default(): + names = sorted(n.name if isinstance(n, ast.FunctionDef) else n.target.id + for n in _ours().body if isinstance(n, (ast.FunctionDef, ast.AnnAssign)) + and (isinstance(n, ast.FunctionDef) or n.value is not None)) + assert names == ["FINAL_PERIOD", NAME] + assert _final_period(_ours()) is None + + +@pytest.mark.parametrize("sport", sorted(FINAL_PERIOD)) +def test_every_remaining_plugin_copy_matches(sport): + root = _plugins_root() + source = (root / f"{sport}-scoreboard" / "sports.py").read_text(encoding="utf-8") + live = _class(ast.parse(source), "SportsLive") + copy = _method(live) + if copy is None: + assert sports_game_over.__name__ in source, ( + f"{sport}: no {NAME} on SportsLive and no {sports_game_over.__name__} import") + else: + assert _fingerprint(copy) == _fingerprint(_method(_ours())), ( + f"{NAME} in {sport} differs from sports_game_over. " + f"Port the change to both, or stop treating it as shared.") + + +@pytest.mark.parametrize("sport", sorted(FINAL_PERIOD)) +def test_every_plugin_declares_its_final_period(sport): + root = _plugins_root() + source = (root / f"{sport}-scoreboard" / "sports.py").read_text(encoding="utf-8") + assert _final_period(_class(ast.parse(source), "SportsLive")) == FINAL_PERIOD[sport] + + +def test_the_drift_report_still_calls_it_identical(): + root = _plugins_root() + families = DRIFT.build(root, ("sports.py",)) + rows = {(r["file"], r["family"]): r + for r in (DRIFT.summarise(k, v) for k, v in families.items())} + row = rows.get(("sports.py", NAME)) + assert row is None or row["worst_class_variants"] == 1 From 6fb2dc35951de94740c64c08a11cfeb2f2a08600 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Mon, 5 Oct 2026 09:52:22 -0400 Subject: [PATCH 3/8] fix(sports): honour every pending kickoff in the idle back-off, not just the first (#772) _note_scheduled_start_candidate kept one kickoff. While it was inside its 15-minute grace every later kickoff was refused, and by the time the grace ended the later one had passed and was refused again as already past. So of two favourites kicking off within 15 minutes of each other, the second lost its own grace: if the first game was not live by then (a rain delay, a postponement, ESPN slow to flip it) and ESPN had not flipped the second either, the back-off went straight back to its ceiling and the second game was noticed up to that late. Later kickoffs now wait in a short queue (_later_scheduled_starts, the earliest 8). When the current kickoff's grace ends, the earliest queued one still inside its own grace takes over -- including one that has already passed. A kickoff still holds the live cadence for at most its own grace, so a postponed game costs the same quarter of an hour as before, and _next_scheduled_start_ts keeps its meaning for anything that reads or sets it. The promotion is a module function, so the mixin's method set is unchanged. Table tests replay the idle loop on a fake clock over kickoff schedules; mutation-checked (the old code fails 10 of the new tests). Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 14 ++++ src/common/sports_shared.py | 58 ++++++++++++++-- test/test_sports_shared.py | 130 ++++++++++++++++++++++++++++++++++++ 3 files changed, 196 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0a260dad..2534fd02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -194,6 +194,20 @@ Internal; no behaviour change. Stage 3 of `docs/RUN_LOOP_REDESIGN.md`. (`refresh_registry_in_background()`, backing off for a minute after an offline failure), so a later load has them. The store, install and update paths still fetch as before. +- A sports live manager's idle back-off now honours every pending kickoff, + not just the first. `_note_scheduled_start_candidate()` kept one kickoff + and, while it was inside its 15-minute grace, refused every later one; by + the time the grace ended the later one had passed and was refused again. + So of two favourites kicking off within 15 minutes of each other, the + second lost its own grace: if the first game was not live by then (a rain + delay, a postponement, ESPN slow to flip it) and ESPN had not flipped the + second either, the back-off went back to its ceiling and the second game + was noticed up to the ceiling (15 minutes by default) late. Later + kickoffs now wait in a short queue (`_later_scheduled_starts`, the + earliest 8) and each takes over with a grace of its own when the one + before it expires. A kickoff still + holds the live cadence for at most its own grace, so a postponed game + costs the same quarter of an hour as before. ### ESPN date-range fetches: fewer requests, fewer at once diff --git a/src/common/sports_shared.py b/src/common/sports_shared.py index f1efbbd7..e6d6403d 100644 --- a/src/common/sports_shared.py +++ b/src/common/sports_shared.py @@ -122,6 +122,32 @@ _DEFAULT_LIVE_IDLE_MAX_SECONDS = 900 _KICKOFF_GRACE_SECONDS = 900 #: Fallback cadence around a kickoff when the manager has no update_interval. _KICKOFF_POLL_FLOOR = 30 +#: How many kickoffs after the current one a live manager remembers. Only the +#: earliest few can matter before the next look refreshes the list, so this +#: bounds the memory without dropping a kickoff the board would wait for. +_KICKOFF_QUEUE_MAX = 8 + + +def _current_scheduled_start(host: Any, now: float) -> Optional[float]: + """The kickoff a live manager is honouring now, promoting the next queued one. + + ``_next_scheduled_start_ts`` is the kickoff being honoured: the earliest + one ahead of us, or one that has just passed and is inside its grace. + Kickoffs behind it wait in ``_later_scheduled_starts``. When the current + one's grace runs out, the earliest queued kickoff that is not itself past + its grace takes over -- including one that has already passed, so a + second kickoff inside the first one's grace still gets a grace of its own. + """ + current: Optional[float] = getattr(host, "_next_scheduled_start_ts", None) + if current and current > now - _KICKOFF_GRACE_SECONDS: + return current + queued: Optional[List[float]] = getattr(host, "_later_scheduled_starts", None) + if queued: + alive = sorted(s for s in queued if s > now - _KICKOFF_GRACE_SECONDS) + current = alive.pop(0) if alive else None + host._later_scheduled_starts = alive + host._next_scheduled_start_ts = current + return current if current and current > now - _KICKOFF_GRACE_SECONDS else None def _resolve_font_path(path: str) -> str: @@ -1296,11 +1322,11 @@ class SportsLiveSharedMixin: otherwise look like another empty check and escalate the back-off again, right when the game is actually starting. """ - start = getattr(self, "_next_scheduled_start_ts", None) + now = time.time() + start = _current_scheduled_start(self, now) if not start: return interval live = getattr(self, "update_interval", None) or _KICKOFF_POLL_FLOOR - now = time.time() if now < start: return max(live, min(interval, int(start - now))) if now - start <= _KICKOFF_GRACE_SECONDS: @@ -1315,6 +1341,16 @@ class SportsLiveSharedMixin: already has. Self-correcting: a stored start that has passed is replaced by the next one offered, so a postponed game cannot pin the cadence to a kickoff that never happens. + + Every pending kickoff is honoured, not just the first. A kickoff that + arrives while an earlier one is inside its grace is queued in + ``_later_scheduled_starts`` (the earliest _KICKOFF_QUEUE_MAX of them) + and takes over when that grace ends, with a grace of its own. Keeping + only the one kickoff dropped the second of two favourites starting + within the grace of each other: it was refused while the first held + the slot, and refused again once it had passed, so if ESPN had not + flipped it live by the end of the first grace the back-off went + straight back to its ceiling and the game was noticed up to that late. """ if not isinstance(details, dict): return @@ -1331,7 +1367,7 @@ class SportsLiveSharedMixin: now = time.time() if candidate <= now: return - current = getattr(self, "_next_scheduled_start_ts", None) + current = _current_scheduled_start(self, now) # A kickoff that has only just passed is *kept*, not replaced by the # next one on the card. Replacing it immediately is what made the grace # window in _clamp_to_scheduled_start dead code: the moment 13:00 came @@ -1341,10 +1377,20 @@ class SportsLiveSharedMixin: # polled at 13:00:45, found nothing live because ESPN had not flipped # the status yet, and then went quiet for the next quarter of an hour, # which is the behaviour this whole clamp exists to prevent. - if (current is None - or current <= now - _KICKOFF_GRACE_SECONDS - or candidate < current): + # + # Nor is it forgotten: whichever kickoff loses is queued behind the + # one honoured now, so it gets its own grace when that one's ends. + if current is None: self._next_scheduled_start_ts = candidate + return + if candidate == current: + return + if candidate < current: + self._next_scheduled_start_ts, candidate = candidate, current + queued = getattr(self, "_later_scheduled_starts", None) or [] + if candidate not in queued: + self._later_scheduled_starts = sorted( + [*queued, candidate])[:_KICKOFF_QUEUE_MAX] #: How long a game that finished live is still reported by #: finished_games_snapshot(): long enough for the recent-games list, which diff --git a/test/test_sports_shared.py b/test/test_sports_shared.py index 1cdd6fc2..b9ee0a28 100644 --- a/test/test_sports_shared.py +++ b/test/test_sports_shared.py @@ -465,6 +465,136 @@ class TestLiveMixin: # The safety property that makes it correct: 30s beats 600s. assert h._idle_live_interval() == h.update_interval + # ---- every pending kickoff is honoured, not just the first ------------ + # + # One stored kickoff held the slot through its grace and refused every + # later one; a later one that had passed by the time the grace ended was + # refused again as "already past". So of two favourites kicking off ten + # minutes apart, the second lost its grace: if ESPN had not flipped it live + # by the end of the FIRST game's grace, the back-off returned to its + # ceiling and the game was noticed up to that late. That bites whenever the + # first game is not live by then -- a rain delay, a postponement, ESPN slow + # to flip it -- since a live first game keeps the live cadence anyway. + + @staticmethod + def _replay(monkeypatch, kickoffs, flips, until, poll=30, ceiling=900): + """Drive a live manager's idle loop over a schedule on a fake clock. + + ``kickoffs`` are start offsets in seconds from t=0 (the first look), + ``flips`` how long after its start ESPN reports each game live (None: + postponed, never live). Every + look offers each not-yet-live game, as the live loop does, then sleeps + for whatever the back-off returns. Returns, per game, how long after it + went live it was first seen live -- None if never. + """ + clock = [1_800_000_000.0] + monkeypatch.setattr(sports_shared.time, "time", lambda: clock[0]) + h = _LiveHost(no_data_interval=300) + h.live_idle_max_interval = ceiling + h.update_interval = poll + h._empty_live_streak = 30 # idle all morning: at the ceiling + t0 = clock[0] + seen = [None] * len(kickoffs) + while clock[0] - t0 < until: + now = clock[0] - t0 + live = [f is not None and k + f <= now for k, f in zip(kickoffs, flips)] + for i, is_live in enumerate(live): + if is_live and seen[i] is None: + seen[i] = now - (kickoffs[i] + flips[i]) + start = datetime.fromtimestamp(t0 + kickoffs[i], tz=timezone.utc) + h._note_scheduled_start_candidate( + {"is_live": is_live, "is_halftime": False, + "start_time_utc": start}) + # A real board keeps polling at the live cadence while anything is + # live; a game here stays live for an hour after it flips. + on = any(f is not None and k + f <= now < k + f + 3600 + for k, f in zip(kickoffs, flips)) + h._note_live_fetch(on) + clock[0] += poll if on else h._idle_live_interval() + assert len(getattr(h, "_later_scheduled_starts", None) or ()) <= sports_shared._KICKOFF_QUEUE_MAX + return seen + + @pytest.mark.parametrize("kickoffs,flips", [ + # (start offsets, ESPN's flip delay per game), both in seconds. + pytest.param([1800], [120], id="one kickoff, flipped 2 min late"), + pytest.param([1800, 2400], [0, 840], + id="first live on time, the second flipped 14 min late"), + pytest.param([1800, 2400], [None, 840], + id="first postponed, the second 10 min later flipped 14 min late"), + pytest.param([1800, 2400], [1200, 840], + id="first in a 20 min delay, the second flipped 14 min late"), + pytest.param([1800, 2100, 2520], [None, None, 600], + id="three inside one grace, the last flipped 10 min late"), + pytest.param([1800, 1800, 2400], [None, None, 700], + id="two at the same time, then one 10 min later"), + pytest.param([1800, 2700], [None, 840], + id="second kickoff 15 min later, flipped 14 min late"), + pytest.param([1800 + 60 * i for i in range(20)], + [None] * 19 + [840], + id="twenty kickoffs a minute apart overflow the queue"), + ]) + def test_every_pending_kickoff_gets_its_grace(self, monkeypatch, kickoffs, flips): + seen = self._replay(monkeypatch, kickoffs, flips, + until=max(kickoffs) + 3 * 3600) + late = [s for s, f in zip(seen, flips) + if f is not None and (s is None or s > 30)] + assert not late, "games noticed late (s after going live): %r" % (seen,) + + def test_a_kickoff_that_never_flips_costs_one_grace_then_backs_off(self, monkeypatch): + # A postponed game keeps the live cadence for its grace and no longer: + # remembering more kickoffs must not pin the poll to dead ones. + clock = [1_800_000_000.0] + monkeypatch.setattr(sports_shared.time, "time", lambda: clock[0]) + h = self._idle_host() + t0 = clock[0] + for offset in (600, 900): + h._note_scheduled_start_candidate( + {"start_time_utc": datetime.fromtimestamp(t0 + offset, tz=timezone.utc)}) + clock[0] = t0 + 900 + sports_shared._KICKOFF_GRACE_SECONDS - 1 + assert h._idle_live_interval() == h.update_interval + clock[0] = t0 + 900 + sports_shared._KICKOFF_GRACE_SECONDS + 1 + assert h._idle_live_interval() == 900 + assert not getattr(h, "_later_scheduled_starts", None) + + def test_a_queued_kickoff_past_its_own_grace_is_skipped(self, monkeypatch): + # After a long sleep (or a run of looks that never woke the manager) + # several queued kickoffs may have gone stale at once. The one still + # inside its grace must win, not the first stale one in the queue. + clock = [1_800_000_000.0] + monkeypatch.setattr(sports_shared.time, "time", lambda: clock[0]) + h = self._idle_host() + t0 = clock[0] + for offset in (600, 700, 1500): + h._note_scheduled_start_candidate( + {"start_time_utc": datetime.fromtimestamp(t0 + offset, tz=timezone.utc)}) + clock[0] = t0 + 1500 + 150 # 600 and 700 are past their grace + assert h._idle_live_interval() == h.update_interval + assert h._next_scheduled_start_ts == t0 + 1500 + + def test_the_queue_keeps_the_earliest_kickoffs(self, monkeypatch): + clock = [1_800_000_000.0] + monkeypatch.setattr(sports_shared.time, "time", lambda: clock[0]) + h = self._idle_host() + t0 = clock[0] + cap = sports_shared._KICKOFF_QUEUE_MAX + for offset in reversed(range(1, cap + 6)): # latest first + h._note_scheduled_start_candidate( + {"start_time_utc": datetime.fromtimestamp(t0 + 600 * offset, tz=timezone.utc)}) + assert h._next_scheduled_start_ts == t0 + 600 + assert h._later_scheduled_starts == [t0 + 600 * i for i in range(2, cap + 2)] + + def test_a_kickoff_offered_twice_is_kept_once(self, monkeypatch): + clock = [1_800_000_000.0] + monkeypatch.setattr(sports_shared.time, "time", lambda: clock[0]) + h = self._idle_host() + t0 = clock[0] + for _ in range(3): + for offset in (600, 1200): + h._note_scheduled_start_candidate( + {"start_time_utc": datetime.fromtimestamp(t0 + offset, tz=timezone.utc)}) + assert h._next_scheduled_start_ts == t0 + 600 + assert h._later_scheduled_starts == [t0 + 1200] + def test_finding_a_live_game_resets_the_streak(self): h = _LiveHost() h._note_live_fetch(False) From c20c0beac283fd3ebc7bc98edba348c8971a752d Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Mon, 5 Oct 2026 09:53:43 -0400 Subject: [PATCH 4/8] feat(web): the Display tab is an ES-module page, with a page-visibility service (stage 4) (#771) * feat(web): the Display tab is an ES-module page, with a page-visibility service (stage 4) - core/visibility.js: each page gets ctx.visibility (whileVisible, every, isVisible). Work registered there runs only while the page's tab is active and the browser tab visible, and ends when the page is swapped out. It reads the active tab from window.LEDVisibility, so it agrees with the classic partials. The registry gained a mountContext option for per-mount services. - pages/display.js replaces display.html's two inline scripts. The 5 s sync status poll runs through ctx.visibility.every; the status and scroll-speed hint requests go through ctx.api with ctx.signal, as does the Vegas order widget's plugin-list request. The Advanced toggle is a delegated data-action; window.updateSyncUI is a deprecated alias. - New DOM suites test_visibility_service.js and test_display_page.js; test_display_partial_ids.js imports the module. Co-Authored-By: Claude Opus 5.5 * refactor(web): no computed keys in the Display page's readout and destroy Codacy's object-injection rule flagged v[id] and ctx.state[name]. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 28 ++ docs/WEB_FRONTEND_ARCHITECTURE.md | 42 ++- test/js/README.md | 4 +- test/js/dom/test_display_page.js | 325 ++++++++++++++++++ test/js/dom/test_visibility_service.js | 211 ++++++++++++ test/js/run_all.js | 3 +- test/js/unit/test_display_partial_ids.js | 144 ++++---- test/js/unit/test_html_escaping.js | 9 +- test/js/unit/test_page_registry.js | 40 +++ test/web_interface/test_es_modules.py | 5 +- web_interface/static/v3/js/core/boot.js | 6 + web_interface/static/v3/js/core/registry.js | 15 +- web_interface/static/v3/js/core/visibility.js | 144 ++++++++ web_interface/static/v3/js/pages/display.js | 324 +++++++++++++++++ .../templates/v3/partials/display.html | 301 +--------------- 15 files changed, 1223 insertions(+), 378 deletions(-) create mode 100644 test/js/dom/test_display_page.js create mode 100644 test/js/dom/test_visibility_service.js create mode 100644 web_interface/static/v3/js/core/visibility.js create mode 100644 web_interface/static/v3/js/pages/display.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 2534fd02..f0ba87e2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,34 @@ accepts both, but the store flags the old spelling as deprecated ## Unreleased +### Web UI: the Display tab is an ES-module page, with a page-visibility service (stage 4) + +- New `static/v3/js/core/visibility.js`: each page module gets + `ctx.visibility` with `whileVisible(start, stop)`, `every(ms, fn)` and + `isVisible()`. Work registered there runs only while the page's tab is the + active tab and the browser tab is visible, and ends when the page is + swapped out, with no teardown code in the page. It reads the active tab + from `window.LEDVisibility`, so it agrees with the classic partials that + still use that directly. The page registry gained a `mountContext` option + for services bound to one mounted page. +- The Display tab's inline scripts are now `static/v3/js/pages/display.js`. + The partial has no inline script, `onclick` or `onchange` any more. The + multi-display sync status poll (every 5 s) runs through + `ctx.visibility.every`; the status and scroll-speed hint requests go + through `core/api.js` with the page's abort signal, and so does the Vegas + order widget's plugin-list request. +- Behaviour differences: with sync on, opening the tab asks for the status + once instead of twice, and a Display tab loaded while not on screen waits + until it is. A login redirect during a poll no longer flashes "Sync status + unavailable". The `window.syncStatusInterval` timer id is gone (nothing + read it). A pending scroll-hint request or widget retry is dropped when + the partial is swapped out. +- `window.updateSyncUI` keeps working as a deprecated alias through + `window.LEDMatrix` (one console warning). +- New suites `test/js/dom/test_visibility_service.js` and + `dom/test_display_page.js`; `unit/test_display_partial_ids.js` imports the + module instead of slicing the template. + ### Plugins ask for the screen in-process: `request_on_demand()` / `end_on_demand()` The in-process way in that stage 5 of the control socket needed diff --git a/docs/WEB_FRONTEND_ARCHITECTURE.md b/docs/WEB_FRONTEND_ARCHITECTURE.md index b7ae12ba..962107da 100644 --- a/docs/WEB_FRONTEND_ARCHITECTURE.md +++ b/docs/WEB_FRONTEND_ARCHITECTURE.md @@ -42,7 +42,8 @@ static/v3/js/ registry.js page lifecycle: init/destroy on htmx swaps api.js fetch wrapper for /api/v3 (JSON envelope, login redirect) facade.js window.LEDMatrix and deprecated aliases - (later) escape.js, notify.js, dialog.js, streams.js, visibility.js, + visibility.js ctx.visibility: a page's timers run only while it is on screen + (later) escape.js, notify.js, dialog.js, streams.js, store.js (the one installed-plugin store), form/renderer.js pages/ one module per tab partial cache.js export init(root, ctx), destroy(root, ctx) @@ -81,6 +82,12 @@ The conventions the converted pages share: to the module's export of the same name and warns once. - **Timers are cleared in `destroy()`**, the one thing `ctx.signal` cannot undo by itself. +- **Polling goes through `ctx.visibility`.** A refresh that repeats + (`ctx.visibility.every(ms, fn)`) or work that should run only while the + page is on screen (`ctx.visibility.whileVisible(start, stop)`) is + registered there, never with a bare `setInterval`. It runs only while the + page's tab is the active tab and the browser tab is visible, and it ends + when the page is destroyed, with no code in `destroy()`. - **A page reports its own htmx saves.** A form whose result a page module shows (an `htmx:afterRequest` listener on the page root, in place of an `hx-on` attribute naming a global) carries `data-reports-result`. `app.js` @@ -111,6 +118,7 @@ Each mount gets a `ctx` object: | `ctx.state` | A per-mount object for the page's own state | | `ctx.api` | Shared service from `boot.js` | | `ctx.notify` | Shared service from `boot.js` | +| `ctx.visibility` | This page's handle on `core/visibility.js` (below), made per mount by `boot.js` through the registry's `mountContext` option | A page that passes `{ signal: ctx.signal }` to `addEventListener` and `fetch` needs no teardown code. Its listeners and in-flight requests go @@ -119,6 +127,30 @@ example: its delete buttons use one delegated listener, rows are built with `textContent` rather than markup strings, and a newer load supersedes an older one. +### Page visibility + +`core/visibility.js` gives each mounted page `ctx.visibility`: + +| Member | What it does | +|---|---| +| `whileVisible(start, stop)` | Runs `start()` when the page comes on screen (at once, if it mounts on screen) and `stop()` when it leaves. Returns a function that ends the registration, running `stop()` first if needed | +| `every(ms, fn)` | `fn()` at once, then every `ms` while on screen. The interval is cleared while hidden and restarted, with an immediate `fn()`, when the page is back. Returns the same kind of end function | +| `isVisible()` | True while the page is on screen | +| `tab` | The tab the page belongs to: its name, or `forPage(ctx, { tab })` | + +"On screen" means the page's tab is the active tab and the browser tab is +visible. Everything a page registered ends when its `ctx.signal` aborts, +after `destroy()`, so a swapped-out partial leaves no interval behind. + +The answer comes from `window.LEDVisibility` (`app-shell.js`), read at call +time, so the page modules and the classic partials that still call it +(Overview, Logs, Tools) agree on the active tab, and the SSE streams keep +pausing with them. Each registration takes its own `LEDVisibility` key, so +registrations never replace each other or a classic partial's. Without +`LEDVisibility` (a page outside `base.html`), the browser tab's visibility +alone decides. Moving the tracker itself into the module (the shell table +below) changes only `core/visibility.js`. + ### One facade `window.LEDMatrix` is the only global the module code adds: @@ -249,7 +281,7 @@ are the inline script in each partial today. | 5 | Backup & Restore | 232 lines, now 0 | **Done in stage 2.** Its 5 globals (`exportBackup`, `loadBackupList`, `validateRestoreFile`, `clearRestore`, `runRestore`) are deprecated aliases; the buttons are delegated `data-action`s. Uploads go through `ctx.api.request(..., { body: formData })` (`api.js` gained a raw `body` option) | | 6 | Schedule | 193 lines, now 0 | **Done in stage 3.** Its 2 `hx-on` response handlers (`handleScheduleResponse`, `handleDimScheduleResponse`) are one `htmx:afterRequest` listener on the page root, and deprecated aliases. The forms are marked `data-reports-result` so `app.js` does not repeat the server's message. The saved schedules reach the module as JSON in `data-schedule-config` / `data-dim-schedule-config` instead of being templated into the script | | 7 | General | 153 lines, now 0 | **Done in stage 3.** The Security section's three forms and two buttons are delegated `data-action`s (one submit and one click listener); `window.webLogin` is a deprecated alias of an object with its five methods. Login requests go through `ctx.api`, so the login redirect is quiet. The settings form keeps its `hx-on` call to the shared `showSaveResult`, as Rotation's does | -| 8 | Display | 231 | First page with `LEDVisibility` timers: those move to a `ctx.visibility` service that stops on destroy | +| 8 | Display | 292 lines (2 scripts), now 0 | **Done in stage 4.** The first page with a timer: the 5 s multi-display sync poll is `ctx.visibility.every(5000, ...)` (above), so it runs only while the tab is on screen and stops when the partial is swapped out. Its one global, `updateSyncUI` (the Role menu's `onchange`), is a deprecated alias; the Advanced section's `onclick` is a delegated `data-action="toggle-section"` that calls the shared `toggleSection`. The status poll and the scroll-speed hint go through `ctx.api` with `ctx.signal`, as does the Vegas order widget's plugin-list request. The settings form keeps its `hx-on` call to `showSaveResult` and its `onsubmit` call to `fixInvalidNumberInputs`, as Rotation's does | | 9 | Overview | 410 (4 scripts) | First-run surface: Getting Started, update banner, live preview. Five globals | | 10 | WiFi | 364 | `x-data="wifiSetup()"` is defined by its own script. Moves to `Alpine.data()` registered from the module. AP-mode first screen, so it needs the AP-mode test on a real device | | 11 | Fonts | 681 | Large, but self-contained (6 globals) | @@ -266,7 +298,7 @@ the order: | `showNotification` | 4 versions | `core/notify.js` | | The modal helper | `utils/dialog.js` | `core/dialog.js` | | SSE streams | `app-shell.js` | `core/streams.js` | -| `LEDVisibility` | `app-shell.js` | `core/visibility.js` | +| `LEDVisibility` | `app-shell.js` | `core/visibility.js` (the page-facing `ctx.visibility` is there since step 8; it reads the tracker from `app-shell.js`) | Each move leaves the old global as an alias. When the last inline script is gone, the script re-execution in `htmx-config.js` and the "HTMX never @@ -286,13 +318,15 @@ Unit suites need only node. They import the shipped modules directly: | Suite | Kind | What it covers | |---|---|---| -| `unit/test_page_registry.js` | Unit, minimal DOM shim | The lifecycle: one init per root, destroy on swap, a veto keeps the page, swaps elsewhere leave it alone, the sweep, lazy loading, a destroy while loading, error containment | +| `unit/test_page_registry.js` | Unit, minimal DOM shim | The lifecycle: one init per root, destroy on swap, a veto keeps the page, swaps elsewhere leave it alone, the sweep, lazy loading, a destroy while loading, error containment, `mountContext` fields per mount | +| `dom/test_visibility_service.js` | DOM: real `LEDVisibility` from `app-shell.js`, real registry, no server | `whileVisible` and `every` start and stop with the active tab and the browser tab's visibility; no interval runs while hidden or after a swap-out; one interval after five swaps; registrations never replace each other or a classic partial's; a destroyed page registers nothing; a throwing `start()` is contained; the no-`LEDVisibility` fallback | | `unit/test_core_modules.js` | Unit | `api.js` (envelope, errors, abort, login redirect, path check) and `facade.js` (facade, aliases) | | `dom/test_cache_page.js` | DOM: real partial, real API shape | No inline script; one request per swap and per Refresh after five swaps; a cancelled request draws nothing; hostile keys stay text; delete, empty, error, network and login states | | `dom/test_durations_page.js` | DOM: real partial, real widget, real API shape | One plugin-list request per swap; Move down moves one place after five swaps; the swap cancels a request in flight; a late-loading widget is waited for, and a page swapped away while waiting starts nothing; hostile names stay text | | `dom/test_operation_history_page.js` | DOM: real partial, real API shape | One history request per swap and per Refresh; the plugin filter filled once (from `PluginAPI`'s cache when loaded); paging, filters, debounced search, Clear (one DELETE), error/network/login states, cancel on swap; hostile ids, users and errors stay text | | `dom/test_raw_json_page.js` | DOM: real partial, real config | One POST per Save after five swaps, to the right file; Format and Validate act once; invalid JSON never sent and its message stays text; a save survives a swap and is still reported; the old globals' entry points | | `dom/test_schedule_page.js` | DOM: real partial, real widget | Both pickers drawn once per swap from the saved config; after five swaps each form's answer is one notification (message, fallback, refused, non-JSON, `null`), a request from outside the forms none; the brightness label; a late widget waited for, a page swapped away while waiting draws nothing; the old globals' entry points | +| `dom/test_display_page.js` | DOM: real partial, real widget, real `LEDVisibility`, real API shape | After five swaps one page, one sync interval, the Vegas order drawn once and each control acting once (brightness, resolution, the two show/hide toggles, the Advanced toggle, one debounced hint request); the sync poll only while on screen and never after a swap-out; sync states and hostile peer names as text, failure and login answers; a late widget waited for; `updateSyncUI`'s entry point | | `dom/test_general_page.js` | DOM: real partial, real widget, real API shape | The timezone picker drawn once per swap with the saved zone; the settings form left to htmx; after five swaps each Security action makes one request (create, copy, revoke and its cancel, password and its mismatch); hostile token names stay text; refused, network and login answers; a create made before a swap is still reported and draws nothing; `webLogin`'s entry points | | `dom/test_backup_restore_page.js` | DOM: real partial, real API shape | One request per Refresh, Delete, Export (busy button ignores a second click), Inspect and Restore after five swaps; the upload's fields and the six restore options; reads cancelled by a swap, writes not; hostile file and host names stay text; the old globals' entry points | | `test/web_interface/test_es_modules.py` | pytest | MIME type; `no-cache` without `?v` and immutable with it; `boot.js` loads last; every import resolves inside `core/` and `pages/`; the converted pages are exactly the registered ones, each with its module, `init`, and one root in the rendered partial; a converted partial has no ` @@ -830,7 +795,7 @@ With this off a live game takes over the whole display with the full-screen scor
- @@ -879,261 +844,3 @@ With this off a live game takes over the whole display with the full-screen scor
- - From a669d781f5a7d402f82c623a2ac42dd600eeab20 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Mon, 5 Oct 2026 10:11:17 -0400 Subject: [PATCH 5/8] chore: prepare the 3.8.1 release (#756) * chore: prepare the 3.8.1 release Bumps src.__version__ to 3.8.1 and moves the Unreleased CHANGELOG entries under a 3.8.1 heading, with an empty Unreleased above it. The reason for the release is smooth scrolling at held-frame speeds. On 3.8.0 the default 50 px/s snapped to a stepped 48 px/s on a 120 Hz panel, and any scroll slower than one pixel per refresh showed a half-pixel step across the middle of the panel. Both are fixed on main (#710, #711) but were in no release, so every stable-channel device still had them. - #710's CHANGELOG entry had been filed under 3.8.0 although it merged after the v3.8.0 tag; it moves to 3.8.1. - #711 had no CHANGELOG entry; it gets one. scripts/check_release_version.py v3.8.1 passes. Co-Authored-By: Claude Opus 5.5 * docs: sports_game_over and draw_text_outlined ship in 3.8.1 Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 50 ++++++++++++++++++++++++++++---------- docs/SPORTS_UNIFICATION.md | 2 +- src/__init__.py | 2 +- src/common/README.md | 4 +-- 4 files changed, 41 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f0ba87e2..abeee525 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,42 @@ accepts both, but the store flags the old spelling as deprecated ## Unreleased +## 3.8.1 + +Smooth scrolling at the slower speeds, and the fixes and performance work +since 3.8.0. Highlights: the default 50 px/s and every other held-frame speed +now scroll cleanly (below), Raspberry Pi OS Bookworm is supported alongside +Trixie, updates refresh the systemd units, the display control socket gains +stages 2 and 3, the shared fetch service lands (stages 1 and 2), and a run of +web UI and Plugin Manager fixes. One new module is for plugins: +`src.common.sports_game_over` (sports family 5), which the scoreboards adopt +by flooring on 3.8.1; the other new modules are core-internal and set no +`ledmatrix_min_version` floor. + +### Scroll speed + +These two entries were the reason for this release: on 3.8.0 a slow scroll +either stepped or showed a half-pixel tear across the middle of the panel, +so only speeds of one pixel per refresh looked right. + +- The Vegas Scroll Speed slider now says what the panel will do with the speed + it is on, and offers the nearest smooth ones to click. Only speeds that advance + a whole number of pixels per refresh look smooth, and which those are depends + on the panel (`GET /api/v3/config/scroll-speed-advice`, built on + `scroll_config.speed_advice()`; it uses the refresh the display measured, not + the `limit_refresh_rate_hz` cap). The slider steps by 1 px/s instead of 5. +- The default 50 px/s no longer snaps to a stepped 48 px/s (2 px every 5 + refreshes, 24 fps) on a 120 Hz panel: `solve_crisp()` now prefers 60 or 40 px/s, + which move one pixel at a time. 100 Hz panels are unaffected. (#710) +- A held-frame scroll (one pixel every two or more refreshes, such as 50 or + 60 px/s on a 100-120 Hz panel) no longer shows a half-pixel step across the + middle of the panel. Scan-order compensation ran only at one frame per + refresh; a held frame is now presented as a sequence of swaps + (`scan_order.refresh_plan()`), so the half of the panel that scans later + steps one refresh after the rest. It is skipped when a blit takes more than + half a refresh, since the second blit has to land before the next vsync. + (#711) + ### Web UI: the Display tab is an ES-module page, with a page-visibility service (stage 4) - New `static/v3/js/core/visibility.js`: each page module gets @@ -719,7 +755,7 @@ policies are unchanged. class attribute, `None` by default (the clock never ends a game); the scoreboards declare 3 (hockey), 4 (basketball, football, lacrosse) or `None`. List the mixin before `SportsLiveSharedMixin`. A plugin may import - it once it floors on the release that ships it, and deletes its copy then. + it once it floors on 3.8.1, and deletes its copy then. (#770) ### Tooling @@ -1373,18 +1409,6 @@ guard the import, since the loader's version check is advisory). processes, or turns the socket off with `off`. A non-root dev run uses a private per-user path under the temp directory. -### Scroll speed - -- The Vegas Scroll Speed slider now says what the panel will do with the speed - it is on, and offers the nearest smooth ones to click. Only speeds that advance - a whole number of pixels per refresh look smooth, and which those are depends - on the panel (`GET /api/v3/config/scroll-speed-advice`, built on - `scroll_config.speed_advice()`; it uses the refresh the display measured, not - the `limit_refresh_rate_hz` cap). The slider steps by 1 px/s instead of 5. -- The default 50 px/s no longer snaps to a stepped 48 px/s (2 px every 5 - refreshes, 24 fps) on a 120 Hz panel: `solve_crisp()` now prefers 60 or 40 px/s, - which move one pixel at a time. 100 Hz panels are unaffected. - ### Update channels - Devices no longer pick up every merge to `main`. A new setting, diff --git a/docs/SPORTS_UNIFICATION.md b/docs/SPORTS_UNIFICATION.md index 3c7072f2..2585829c 100644 --- a/docs/SPORTS_UNIFICATION.md +++ b/docs/SPORTS_UNIFICATION.md @@ -91,7 +91,7 @@ more. Shared sports code lives in `src/common`: | `sports_live_scroll.py` | next release | `SportsLiveScrollMixin` — rebuild a live scroll strip mid-cycle, keeping the marquee's place | | `sports_display_rules.py` | next release | `SportsCardOptionsMixin`, `SportsGameRulesMixin` — scorebug date options, the no-favourites filter, non-favourite live dwell | | `sports_font_path.py` | next release | `resolve_font_path` — what the plugins' `_resolve_font_path` copies return | -| `sports_game_over.py` | next release | `SportsGameOverMixin` — `_is_game_really_over`, with the `FINAL_PERIOD` seam (family 5) | +| `sports_game_over.py` | 3.8.1 | `SportsGameOverMixin` — `_is_game_really_over`, with the `FINAL_PERIOD` seam (family 5) | Each is described in [src/common/README.md](../src/common/README.md). diff --git a/src/__init__.py b/src/__init__.py index b181d528..9bf9335e 100644 --- a/src/__init__.py +++ b/src/__init__.py @@ -4,5 +4,5 @@ LEDMatrix Display System Core source package for the LED Matrix Display project. """ -__version__ = "3.8.0" +__version__ = "3.8.1" diff --git a/src/common/README.md b/src/common/README.md index b84198c7..be1d264d 100644 --- a/src/common/README.md +++ b/src/common/README.md @@ -46,7 +46,7 @@ Rules for the package: | [`sports_display_rules`](#sports_display_rules) | Which games a scoreboard shows, for how long, and its scorebug date line | Yes (scoreboards) | 3.8.0 | | [`sports_fetch`](#sports_fetch) | Scoreboard season fetch, lookback and live-odds decisions | Yes (scoreboards) | 3.7.0 | | [`sports_font_path`](#sports_font_path) | Find a scoreboard's bundled font whatever the cwd | Yes (scoreboards) | 3.8.0 | -| [`sports_game_over`](#sports_game_over) | Whether a game ESPN still lists as live has ended | Yes (scoreboards) | next release | +| [`sports_game_over`](#sports_game_over) | Whether a game ESPN still lists as live has ended | Yes (scoreboards) | 3.8.1 | | [`sports_game_renderer`](#sports_game_renderer) | Scoreboard scroll/Vegas card geometry | Yes (scoreboards) | 3.3.0 | | [`sports_helpers`](#sports_helpers) | Small helpers every scoreboard `sports.py` copies | Yes (scoreboards) | 3.5.0 | | [`sports_live_scroll`](#sports_live_scroll) | Rebuild a live scroll strip mid-cycle without moving it | Yes (scoreboards) | 3.8.0 | @@ -402,7 +402,7 @@ Created by `DisplayController`; works with any plugin. `draw_multiline_text()`, `create_text_image()`. `draw_text_outlined(draw, xy, text, font, fill, outline_color=(0, 0, 0), -offsets=OUTLINE_SQUARE)` (Unreleased) draws the text in `outline_color` at +offsets=OUTLINE_SQUARE)` (3.8.1) draws the text in `outline_color` at each offset, then in `fill` on top: the same pixels as one `draw.text` per offset, but the string is rasterized once. `OUTLINE_SQUARE` is the eight-sided one-pixel outline the scoreboards draw, `OUTLINE_CROSS` the From e745ae8060b6381939860ca874c57cf7822d1bf3 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Mon, 5 Oct 2026 10:47:29 -0400 Subject: [PATCH 6/8] feat(display): cap malloc arenas in-process and malloc_trim between screens (#774) * feat(display): cap malloc arenas in-process and malloc_trim between screens Co-Authored-By: Claude Opus 5.5 * test: malloc_tuning with ctypes mocked; add to the mypy ratchet Co-Authored-By: Claude Opus 5.5 * docs(changelog): malloc arena cap and malloc_trim between screens Co-Authored-By: Claude Opus 5.5 * docs(changelog): spacing Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 23 +++++ mypy-clean.txt | 1 + run.py | 6 ++ src/display_controller.py | 7 ++ src/malloc_tuning.py | 123 ++++++++++++++++++++++ test/test_malloc_tuning.py | 206 +++++++++++++++++++++++++++++++++++++ 6 files changed, 366 insertions(+) create mode 100644 src/malloc_tuning.py create mode 100644 test/test_malloc_tuning.py diff --git a/CHANGELOG.md b/CHANGELOG.md index abeee525..44724d01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,29 @@ accepts both, but the store flags the old spelling as deprecated ## Unreleased +### The display hands freed memory back to the OS + +The display process's resident memory climbed in steps for hours while the +data it held stayed flat: glibc keeps what Python frees in per-thread malloc +arenas and returns little of it. `src/malloc_tuning.py` (new, standard library +only, a no-op off Linux/glibc) does two things in-process, so it reaches +devices without re-running the installer: + +- **Arena cap at start-up.** `run.py` calls `mallopt(M_ARENA_MAX, 2)` before any + thread exists, the same cap as the unit's `Environment=MALLOC_ARENA_MAX=2`. + Units installed before that line never got it (systemd runs the copy in + `/etc/systemd/system`); a `MALLOC_ARENA_MAX` in the environment still wins. +- **`malloc_trim(0)` between screens**, at most every 5 minutes, from the top of + the render loop where no frame is being drawn. Measured on a Pi 4: 2-11 ms + per call. + +On ledpi (Pi 4, 192x48, Vegas on, nine plugins, a unit without +`MALLOC_ARENA_MAX`), alternated main / branch / branch / main arms of 2.5 h: +two hours in, resident memory was 551 MB on main (the second main arm was +already at 651 MB after 1 h 44 min) against 412 and 386 MB with this change, +and the 20-minute frame soaks came out at 0.147-0.165% late against main's +0.151-0.188%. + ## 3.8.1 Smooth scrolling at the slower speeds, and the fixes and performance work diff --git a/mypy-clean.txt b/mypy-clean.txt index ffc7a6c5..aa3ba212 100644 --- a/mypy-clean.txt +++ b/mypy-clean.txt @@ -59,6 +59,7 @@ src/ipc/contract.py src/ipc/server.py src/logging_config.py src/logo_downloader.py +src/malloc_tuning.py src/matrix_support.py src/pi5_matrix_support.py src/plugin_system/__init__.py diff --git a/run.py b/run.py index c327d549..af7d2c6c 100755 --- a/run.py +++ b/run.py @@ -14,6 +14,12 @@ project_dir = os.path.dirname(os.path.abspath(__file__)) if project_dir not in sys.path: sys.path.insert(0, project_dir) +# Cap glibc's malloc arenas before any thread exists (arenas already made +# stay): the in-process twin of the unit's MALLOC_ARENA_MAX=2, for units +# installed before that line. A no-op off glibc. See src/malloc_tuning.py. +from src import malloc_tuning +malloc_tuning.cap_arenas() + # Under systemd the watchdog clock is already running, and start-up (plugin # loads, initial updates) takes far longer than the render loop's limit. Widen # it before anything slow is imported; the render loop narrows it again once diff --git a/src/display_controller.py b/src/display_controller.py index 84bfbe8c..44a70f84 100644 --- a/src/display_controller.py +++ b/src/display_controller.py @@ -37,6 +37,7 @@ from concurrent.futures import ThreadPoolExecutor, as_completed # pylint: disab import pytz from src import display_watchdog +from src.malloc_tuning import MallocTrimmer from src.display_arbiter import ( Arbiter, ArbiterInputs, ArbiterState, FramePolicy, ScreenPlan, Source, WifiNotice, live_pick, live_takeover, on_demand_bound, rotation_plan, @@ -4217,6 +4218,7 @@ class DisplayController: logger.info(f"Initial mode set to: {self.current_display_mode} (index: {self.current_mode_index}, total modes: {len(self.available_modes)})") self._publish_current_mode_state() runner = ScreenRunner(_MODULE_CLOCK, _ScreenHost(self), logger) + trimmer = MallocTrimmer() while True: # Arms the watchdog after the first frame -- or after the @@ -4224,6 +4226,11 @@ class DisplayController: # it from then on. display_watchdog.watchdog.loop_pass() + # Between screens, nothing being drawn: every few minutes hand + # the memory glibc is holding for freed images back to the OS + # (src/malloc_tuning.py). A clock read when none is due. + trimmer.maybe_trim() + # Apply plugin enable/disable edits saved via the web UI. The # config-watcher thread only sets the flag; loading/unloading and # rebuilding available_modes happens here on the render thread so diff --git a/src/malloc_tuning.py b/src/malloc_tuning.py new file mode 100644 index 00000000..a2fcf632 --- /dev/null +++ b/src/malloc_tuning.py @@ -0,0 +1,123 @@ +"""Keep glibc's malloc from holding on to memory the display has freed. + +The display process allocates and frees PIL images and numpy buffers all day +from a dozen threads. glibc gives each allocating thread its own malloc arena +(up to 8 x CPU count) and returns little of what is freed inside them to the +OS, so resident memory climbs for hours while the live data stays flat. Two +in-process remedies, both standard library only (ctypes) and both no-ops off +Linux/glibc: + +* :func:`cap_arenas` -- ``mallopt(M_ARENA_MAX, 2)``, the in-process twin of the + unit's ``Environment=MALLOC_ARENA_MAX=2``. Units installed before that line + existed never got it (systemd runs the copy in /etc/systemd/system), so the + process applies it itself. Call it before any other thread starts: arenas + already created stay. A ``MALLOC_ARENA_MAX`` set in the environment wins. +* :class:`MallocTrimmer` -- ``malloc_trim(0)`` at most every few minutes, + called from the render loop between screens, where no frame is being drawn. + glibc 2.8+ releases free pages from the middle of every arena, not only the + top of the main heap. + +Without glibc (macOS, Windows, musl, the dev server on any of them) nothing is +loaded and every call returns False. +""" +import ctypes +import logging +import os +import sys +import time +from typing import Any, Callable, Optional + +logger = logging.getLogger(__name__) + +#: glibc's mallopt() parameter number for the arena cap (malloc.h). +M_ARENA_MAX = -8 + +#: The arena cap applied when the environment does not set one; the same value +#: as the unit's ``MALLOC_ARENA_MAX``. +DEFAULT_ARENA_MAX = 2 + +#: Seconds between malloc_trim() calls. A trim takes about 1-20 ms on a Pi 4, +#: so this keeps it far from frame timing while still returning memory long +#: before it piles up. +TRIM_INTERVAL_SECONDS = 300.0 + +_UNLOADED = object() +_libc: Any = _UNLOADED + + +def _load_libc() -> Optional[Any]: + """The process's C library if it is glibc with malloc_trim, else None.""" + global _libc + if _libc is _UNLOADED: + _libc = None + if sys.platform.startswith('linux'): + try: + libc = ctypes.CDLL(None) + # gnu_get_libc_version is glibc-only, so musl (which has + # mallopt but no malloc_trim) is left alone as a whole. + if all(hasattr(libc, name) for name in + ('gnu_get_libc_version', 'malloc_trim', 'mallopt')): + libc.malloc_trim.argtypes = [ctypes.c_size_t] + libc.malloc_trim.restype = ctypes.c_int + libc.mallopt.argtypes = [ctypes.c_int, ctypes.c_int] + libc.mallopt.restype = ctypes.c_int + _libc = libc + except (OSError, AttributeError, TypeError): + logger.debug("glibc malloc controls unavailable", exc_info=True) + return _libc + + +def cap_arenas(max_arenas: int = DEFAULT_ARENA_MAX) -> bool: + """Cap glibc's malloc arenas at ``max_arenas``. True when the cap was set. + + Skipped when ``MALLOC_ARENA_MAX`` is in the environment: glibc has read it + already, and an operator who set it chose that value. + """ + if os.environ.get('MALLOC_ARENA_MAX'): + return False + libc = _load_libc() + if libc is None: + return False + try: + return bool(libc.mallopt(M_ARENA_MAX, int(max_arenas))) + except Exception: # pylint: disable=broad-except + logger.debug("mallopt(M_ARENA_MAX) failed", exc_info=True) + return False + + +class MallocTrimmer: + """Calls ``malloc_trim(0)`` at most once per ``interval`` seconds. + + :meth:`maybe_trim` is meant for an idle point of the render loop; it costs + one clock read when no trim is due. The first trim comes one interval + after construction, so start-up's allocations have settled. + """ + + def __init__(self, interval: float = TRIM_INTERVAL_SECONDS, + clock: Callable[[], float] = time.monotonic) -> None: + self._interval = interval + self._clock = clock + self._libc = _load_libc() + self._next = clock() + interval + + @property + def available(self) -> bool: + return self._libc is not None + + def maybe_trim(self) -> bool: + """Trim if one is due. True when malloc_trim ran and released memory.""" + if self._libc is None: + return False + now = self._clock() + if now < self._next: + return False + self._next = now + self._interval + try: + released = bool(self._libc.malloc_trim(0)) + except Exception: # pylint: disable=broad-except + logger.debug("malloc_trim failed; not trying again", exc_info=True) + self._libc = None + return False + logger.debug("malloc_trim(0) took %.1f ms, released=%s", + (self._clock() - now) * 1000.0, released) + return released diff --git a/test/test_malloc_tuning.py b/test/test_malloc_tuning.py new file mode 100644 index 00000000..438cce48 --- /dev/null +++ b/test/test_malloc_tuning.py @@ -0,0 +1,206 @@ +"""src/malloc_tuning.py: glibc arena cap and periodic malloc_trim, ctypes mocked.""" +import ctypes +from pathlib import Path +from unittest import mock + +import pytest + +from src import malloc_tuning as mt + + +class FakeLibc: + """Stands in for ctypes.CDLL(None) on glibc: records calls.""" + + def __init__(self, trim_result=1, glibc=True): + self.trims = [] + self.mallopts = [] + self._trim_result = trim_result + if glibc: + self.gnu_get_libc_version = lambda: b'2.41' + self.malloc_trim = mock.Mock(side_effect=self._trim) + self.mallopt = mock.Mock(side_effect=self._mallopt) + + def _trim(self, pad): + self.trims.append(pad) + if isinstance(self._trim_result, Exception): + raise self._trim_result + return self._trim_result + + def _mallopt(self, param, value): + self.mallopts.append((param, value)) + return 1 + + +@pytest.fixture(autouse=True) +def fresh_libc(monkeypatch): + """Each test loads the C library itself; nothing real is called.""" + monkeypatch.setattr(mt, '_libc', mt._UNLOADED) + monkeypatch.delenv('MALLOC_ARENA_MAX', raising=False) + yield + + +def _on_glibc(monkeypatch, libc): + monkeypatch.setattr(mt.sys, 'platform', 'linux') + cdll = mock.Mock(return_value=libc) + monkeypatch.setattr(mt.ctypes, 'CDLL', cdll) + return cdll + + +class Clock: + def __init__(self, t=1000.0): + self.t = t + + def __call__(self): + return self.t + + +# -- loading ---------------------------------------------------------------- + +@pytest.mark.parametrize('platform', ['win32', 'darwin', 'freebsd14']) +def test_not_linux_loads_nothing(monkeypatch, platform): + monkeypatch.setattr(mt.sys, 'platform', platform) + cdll = mock.Mock(side_effect=AssertionError('must not load')) + monkeypatch.setattr(mt.ctypes, 'CDLL', cdll) + assert mt._load_libc() is None + assert mt.cap_arenas() is False + trimmer = mt.MallocTrimmer(interval=0) + assert not trimmer.available + assert trimmer.maybe_trim() is False + cdll.assert_not_called() + + +def test_linux_without_glibc_is_a_noop(monkeypatch): + """musl: no gnu_get_libc_version (and no malloc_trim) -- nothing is called.""" + libc = FakeLibc(glibc=False) + del libc.malloc_trim + _on_glibc(monkeypatch, libc) + assert mt._load_libc() is None + assert mt.cap_arenas() is False + assert mt.MallocTrimmer(interval=0).maybe_trim() is False + assert libc.mallopts == [] + + +def test_cdll_failure_is_a_noop(monkeypatch): + monkeypatch.setattr(mt.sys, 'platform', 'linux') + monkeypatch.setattr(mt.ctypes, 'CDLL', mock.Mock(side_effect=OSError('no libc'))) + assert mt._load_libc() is None + assert mt.cap_arenas() is False + + +def test_loads_once(monkeypatch): + cdll = _on_glibc(monkeypatch, FakeLibc()) + mt._load_libc() + mt._load_libc() + mt.MallocTrimmer() + assert cdll.call_count == 1 + + +def test_declares_c_signatures(monkeypatch): + libc = FakeLibc() + _on_glibc(monkeypatch, libc) + mt._load_libc() + assert libc.malloc_trim.argtypes == [ctypes.c_size_t] + assert libc.mallopt.argtypes == [ctypes.c_int, ctypes.c_int] + + +# -- cap_arenas --------------------------------------------------------------- + +def test_cap_arenas_calls_mallopt(monkeypatch): + libc = FakeLibc() + _on_glibc(monkeypatch, libc) + assert mt.cap_arenas() is True + assert libc.mallopts == [(mt.M_ARENA_MAX, 2)] + assert mt.M_ARENA_MAX == -8 # glibc's malloc.h + + +def test_cap_arenas_defers_to_the_environment(monkeypatch): + libc = FakeLibc() + _on_glibc(monkeypatch, libc) + monkeypatch.setenv('MALLOC_ARENA_MAX', '4') + assert mt.cap_arenas() is False + assert libc.mallopts == [] + + +def test_cap_arenas_swallows_errors(monkeypatch): + libc = FakeLibc() + libc.mallopt = mock.Mock(side_effect=RuntimeError('boom')) + _on_glibc(monkeypatch, libc) + assert mt.cap_arenas() is False + + +def test_cap_arenas_matches_the_unit(): + """The in-process default is the value the unit's MALLOC_ARENA_MAX carries.""" + unit = (Path(__file__).resolve().parent.parent / 'systemd' / 'ledmatrix.service').read_text() + assert f'Environment=MALLOC_ARENA_MAX={mt.DEFAULT_ARENA_MAX}\n' in unit + + +# -- MallocTrimmer ------------------------------------------------------------ + +def test_trim_waits_one_interval_then_rate_limits(monkeypatch): + libc = FakeLibc() + _on_glibc(monkeypatch, libc) + clock = Clock() + trimmer = mt.MallocTrimmer(interval=300, clock=clock) + assert trimmer.available + assert trimmer.maybe_trim() is False # start-up: not yet + clock.t += 299.9 + assert trimmer.maybe_trim() is False + clock.t += 0.1 + assert trimmer.maybe_trim() is True + assert libc.trims == [0] + clock.t += 100 + assert trimmer.maybe_trim() is False # rate-limited + clock.t += 200 + assert trimmer.maybe_trim() is True + assert libc.trims == [0, 0] + + +def test_trim_reports_nothing_released(monkeypatch): + libc = FakeLibc(trim_result=0) + _on_glibc(monkeypatch, libc) + clock = Clock() + trimmer = mt.MallocTrimmer(interval=10, clock=clock) + clock.t += 10 + assert trimmer.maybe_trim() is False + assert libc.trims == [0] + + +def test_trim_failure_disables_trimming(monkeypatch): + libc = FakeLibc(trim_result=RuntimeError('boom')) + _on_glibc(monkeypatch, libc) + clock = Clock() + trimmer = mt.MallocTrimmer(interval=10, clock=clock) + clock.t += 10 + assert trimmer.maybe_trim() is False + clock.t += 10 + assert trimmer.maybe_trim() is False + assert libc.trims == [0] # not retried + assert not trimmer.available + + +# -- wiring ------------------------------------------------------------------- + +def test_run_py_caps_arenas_before_threads(): + """run.py applies the cap before the watchdog or the controller import.""" + src = (Path(__file__).resolve().parent.parent / 'run.py').read_text() + cap = src.index('malloc_tuning.cap_arenas()') + assert cap < src.index('display_watchdog.watchdog.begin_startup()') + assert cap < src.index('from src.display_controller import main') + + +def test_render_loop_trims_between_screens(): + src = (Path(__file__).resolve().parent.parent / 'src' / 'display_controller.py').read_text() + loop = src.index('display_watchdog.watchdog.loop_pass()') + trim = src.index('trimmer.maybe_trim()') + assert loop < trim < src.index('outcome = runner.run(plan, manager_to_display)') + + +@pytest.mark.skipif(not mt.sys.platform.startswith('linux'), reason='glibc only') +def test_real_libc_on_linux(): + """On a real Linux C library the calls go through without raising.""" + if mt._load_libc() is None: + pytest.skip('not glibc') + trimmer = mt.MallocTrimmer(interval=0) + assert trimmer.available + assert trimmer.maybe_trim() in (True, False) + assert trimmer.available # did not fail and disable itself From 3bdb5bff3b7dba88a9f4334f658b46bb087863f5 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Mon, 5 Oct 2026 12:33:05 -0400 Subject: [PATCH 7/8] feat(common): sports_favorites -- the reconciled favourite matching (sports family 6) (#775) * feat(common): sports_favorites -- the reconciled favourite matching (sports family 6) New hardware-free module src/common/sports_favorites.py, copied from ledmatrix-plugins claude/family6-reconcile once the nine scoreboards made _is_favorite_game (seven bodies), _select_games_for_display (two) and _select_recent_games_for_display (three) one body each. One mixin per class that carries the methods, so adopting one gives no manager a method it did not have: - SportsFavoritesMixin (SportsCore): _is_favorite_game and _favorite_code. - SportsUpcomingFavoritesMixin: _select_games_for_display. - SportsRecentFavoritesMixin: _select_recent_games_for_display. Each side of a game is named by the 3.5.0 _favorite_key seam (SportsHelpersMixin; the abbreviation by default, nrl overrides it with the ESPN team id and None for a missing id) and compared with favorite_teams stripped and upper-cased. The selection methods give each favourite up to the per-team limit, count a game between two favourites for both, treat only games with an id as possible duplicates and log their summary at INFO. - test/test_sports_favorites.py: the plugins' pinned cases for an abbreviation host and an id-keyed (nrl-style) host -- case, spaces, ids, the NEW collision, the "None" favourite, missing keys; selection order, limits, duplicates and the id-less fix, the INFO summary; host contract, one carrier per method, and SportsGameRulesMixin reaching the shared body. - test/test_sports_favorites_parity.py: with LEDMATRIX_PLUGINS, compares each body with every plugin copy (drift-report normalisation plus decorators), checks no other plugin class carries a copy, and that only nrl overrides _favorite_key. - mypy ratchet, src/common/README.md, CHANGELOG (Unreleased, New modules). - sports_helpers docstrings: _favorite_key now has a caller and an override. - docs/SPORTS_UNIFICATION.md: family 6 status and decisions, and the seam table. SportsCoreSharedMixin._round_robin_favorites still groups by raw abbreviation or _team_in: it is not one of the plugin bodies, so it waits for a later family. Co-Authored-By: Claude Opus 5.5 * docs(sports): family 6 also routes the Upcoming favourites-only filter and three live boosts ledmatrix-plugins claude/family6-reconcile now sends the Upcoming update()'s favourites-only pre-filter and the basketball, hockey and lacrosse live favourite boost through _is_favorite_game, so a lower-case favourite works on a favourites-only Upcoming board. The module is unchanged (update() is not promoted); the parity test still passes against the branch. Updates the pinned row and cell counts and what is left for later families. Co-Authored-By: Claude Opus 5.5 * docs: cite ledmatrix-plugins #635 for the family 6 reconcile Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 13 ++ docs/SPORTS_UNIFICATION.md | 52 ++++- mypy-clean.txt | 1 + src/common/README.md | 14 ++ src/common/sports_favorites.py | 263 +++++++++++++++++++++++++ src/common/sports_helpers.py | 22 +-- test/test_sports_favorites.py | 280 +++++++++++++++++++++++++++ test/test_sports_favorites_parity.py | 135 +++++++++++++ 8 files changed, 761 insertions(+), 19 deletions(-) create mode 100644 src/common/sports_favorites.py create mode 100644 test/test_sports_favorites.py create mode 100644 test/test_sports_favorites_parity.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 44724d01..90e0e87e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,19 @@ already at 651 MB after 1 h 44 min) against 412 and 386 MB with this change, and the 20-minute frame soaks came out at 0.147-0.165% late against main's 0.151-0.188%. +### New modules + +- `src/common/sports_favorites.py` -- sports consolidation family 6, once the + plugins made `_is_favorite_game` (seven bodies), `_select_games_for_display` + (two) and `_select_recent_games_for_display` (three) one each: + `SportsFavoritesMixin` (`SportsCore`: `_is_favorite_game`, `_favorite_code`), + `SportsUpcomingFavoritesMixin` and `SportsRecentFavoritesMixin` (the + favourites-only picks). Each side of a game is named by the 3.5.0 + `_favorite_key` seam and compared with `favorite_teams` stripped and + upper-cased; nrl overrides the key with the ESPN team id. Only a game with an + id can be a duplicate. A plugin may inherit the mixins once it floors on the + release that ships this module, and deletes its copies then. + ## 3.8.1 Smooth scrolling at the slower speeds, and the fixes and performance work diff --git a/docs/SPORTS_UNIFICATION.md b/docs/SPORTS_UNIFICATION.md index 2585829c..f30e8a4c 100644 --- a/docs/SPORTS_UNIFICATION.md +++ b/docs/SPORTS_UNIFICATION.md @@ -92,6 +92,7 @@ more. Shared sports code lives in `src/common`: | `sports_display_rules.py` | next release | `SportsCardOptionsMixin`, `SportsGameRulesMixin` — scorebug date options, the no-favourites filter, non-favourite live dwell | | `sports_font_path.py` | next release | `resolve_font_path` — what the plugins' `_resolve_font_path` copies return | | `sports_game_over.py` | 3.8.1 | `SportsGameOverMixin` — `_is_game_really_over`, with the `FINAL_PERIOD` seam (family 5) | +| `sports_favorites.py` | next release | `SportsFavoritesMixin`, `SportsUpcomingFavoritesMixin`, `SportsRecentFavoritesMixin` — `_is_favorite_game` and the favourites-only picks, on the `_favorite_key` seam (family 6) | Each is described in [src/common/README.md](../src/common/README.md). @@ -103,7 +104,8 @@ modules taken from the plugin copies, each a **new module** rather than growth on an existing one: a plugin that deletes a method copy and relies on an older module having gained it fails at runtime with an `AttributeError`, while a missing module fails at load, where the version checks can see it. -`sports_helpers.py` holds `_favorite_key`, the override point listed below. +`sports_helpers.py` holds `_favorite_key`, the override point listed below; +`sports_favorites.py` is what calls it. Each promoted module has a parity test that compares its bodies against the plugin copies when `LEDMATRIX_PLUGINS` points at a checkout (`test_sports_helpers.py`, `test_sports_stage3_parity.py`), and @@ -126,7 +128,7 @@ deprecation cycle. | `_custom_scorebug_layout(game, draw)` | Per-sport overlay on the base layout | no-op | | `score_phrase(points, team_abbr)` | Celebration wording (`"GOOOOAAALLL!"` vs `"TOUCHDOWN!"`). `points` is the score delta, which sports with variable-value scores use to name the play | `" SCORES!"` — only consulted when `CelebrationMixin` is present | | `win_phrase(team_abbr)` | Win-celebration wording | `" WINS!"` — mixin only | -| `_favorite_key(game, side)` | Which view-model field identifies a team for favorites matching | `game["_abbr"]` | +| `_favorite_key(game, side)` | Which view-model field identifies a team for favorites matching. `sports_favorites` compares it, and each `favorite_teams` entry, stripped and upper-cased; a `None` matches nothing | `game["_abbr"]`. nrl returns the ESPN team id, `None` when it is missing | | `_config_schema_path()` | Plugin's `config_schema.json` — returning it routes `_get_layout_offset` through the `src.element_style` resolver (and gives it the defaults to compare against) | `None`, i.e. the classic inline `customization.layout` read | | `_font_root()` | Directory to resolve `assets/fonts` against | core install root | @@ -315,6 +317,35 @@ were pixel-identical. `src/common/sports_game_over.py` holds the body; `test/test_sports_game_over_parity.py` compares it, and each plugin's `FINAL_PERIOD`, with the plugin copies. +### Family 6: favourite matching (core done; adoption waits for a release) + +ledmatrix-plugins `scripts/test_favourite_matching.py` (#634) pinned 204 rows +across the nine plugins first: `_is_favorite_game` on each manager role, the +two selection methods, the real `update()` with favourites-only on and off, +and the INFO summary; the reconcile extends it to 217 (a lower-case and a +padded favourite through `update()`, and the live favourite boost). The reconcile (ledmatrix-plugins +#635) made `_is_favorite_game` one body on `SportsCore` +(afl and soccer's `SportsUpcoming` copies and five `SportsLive` copies, all +redundant, are gone), added `_favorite_code` beside it, and gave nrl a +`_favorite_key` override instead of its own copies. So that a lower-case +favourite works on a favourites-only Upcoming board, the Upcoming `update()`'s +favourites-only pre-filter and the basketball, hockey and lacrosse live boost +now ask `_is_favorite_game` too (a one-line change each; `update()` itself is +family 13). Of 3,897 cells only those the decisions above explain changed: +case and spaces in eight plugins (30-40 each), the id-less duplicate fix (6-8 +each), nrl's key (6) and its "None" match (6), and the INFO line in baseball, +football and ufc. The harness renders were byte-identical. `src/common/sports_favorites.py` holds the +bodies, one mixin per carrying class; `test/test_sports_favorites_parity.py` +compares them with the plugin copies and checks that only nrl overrides +`_favorite_key`. + +Left for later families: the live screens' favourites-only filter +(`_classify_live_game` and its inline copies) and favourites-first sort still +compare abbreviations exactly, and +`SportsCoreSharedMixin._round_robin_favorites` groups favourites by raw +abbreviation (or by `_team_in` where a plugin has one) instead of through +`_favorite_key`. The result-colour helpers also wait (decision above). + ### Why the method changes Byte-identical promotion has nearly run dry. Measured on ledmatrix-plugins @@ -400,7 +431,7 @@ release. |---|---|---|---| | 4 | Identical sweep | `manager.py`: `_dispatch_switch_refresh`, `_favorite_team_is_live`, `get_vegas_priority_weight`, `_game_involves`, `_favorite_scan_targets`, `_favorite_scan_games`, `_get_total_games_for_manager` (all nine, 1); the live-scroll helpers `_preserving_scroll_position`, `_refresh_live_scroll_managers`, `_live_scroll_managers`, `_note_live_scroll_built`, `_live_scroll_needs_rebuild`, `_live_scroll_fields` (eight, 1). `sports.py`: `_card_option`, `_filtered_or_all`, `_effective_live_duration`, `_recent_date_text` (eight, 1). 58 identical families in all | Nothing to decide; brings `manager.py` into core as a `SportsPluginHostMixin`. `_resolve_font_path` (identical in nine `sports.py` and eight renderers) becomes `sports_font_path.resolve_font_path`, not `font_layout.resolve_asset_path`, which skips the cwd. Core side done; see [Stage 4](#stage-4-the-identical-sweep-core-done-adoption-waits-for-a-release) | | 5 | Game-over check | `SportsLive._is_game_really_over` (5) | Pure logic, no pixels; one seam, `FINAL_PERIOD`. The pilot for the procedure. Reconciled to one body and promoted as `sports_game_over`; adoption waits for the release that ships it. See [Family 5](#family-5-the-game-over-check-core-done-adoption-waits-for-a-release) | -| 6 | Favourite matching | `_is_favorite_game` (7 across three classes), `_select_games_for_display` (2: nrl), `_select_recent_games_for_display` (3) | Everything that asks "is this a favourite" goes through the 3.5.0 `_favorite_key` seam | +| 6 | Favourite matching | `_is_favorite_game` (7 across three classes), `_select_games_for_display` (2: nrl), `_select_recent_games_for_display` (3) | Everything that asks "is this a favourite" goes through the 3.5.0 `_favorite_key` seam. Reconciled to one body each and promoted as `sports_favorites`; adoption waits for the release that ships it. See [Family 6](#family-6-favourite-matching-core-done-adoption-waits-for-a-release) | | 7 | Other-games rotation | `_by_importance`, `_other_games_window`, `_advance_other_games_if_due` (2 each: football), `_rotate_other_games_on_display` (2: ufc) | One outlier each; football carries two fixes the other eight lack | | 8 | Rankings | `_fetch_team_rankings` (3), `_choose_poll` (3), `_load_division_team_ids`, `_passes_other_filters`, `_best_rank`, `_is_ranked_game` (2 each: football) | Needs 7; the rank badge and the "ranked only" filter read it | | 9 | Live fetch and odds | `_fetch_todays_games` (5), `_fetch_odds` (3), `_attach_odds_to_rotated_games` (3) | The prerequisite for one shared ESPN poller across plugins | @@ -444,10 +475,17 @@ suspected behaviour that needs a payload or a rig to confirm first. as `0:00`). A score level at 0:00 is not over: the game stays live through the break before overtime, and one that really ends tied ends on its final status. Baseball keeps its postponed/suspended override in `BaseballLive`. -- **6, favourite matching.** NRL keeps matching favourites by team id - (abbreviations collide: NEW, CAN), through `_favorite_key` rather than its - own copies of the selection methods. Six plugins log the recent-games - selection at INFO; baseball, football and ufc do not. +- **6, favourite matching. Decided 2026-10-05, done:** each side of a game is + named by `_favorite_key` (the abbreviation; NRL overrides it with the ESPN + team id, and `None` for a missing id, which fixes a favourite typed "None" + matching every game without one) and compared with `favorite_teams` + stripped and upper-cased, so " bos" matches BOS. NRL's ambiguous "NEW" + still matches nothing and is logged; routing the result-colour helpers + (`side_is_favorite`, which tint both NEW clubs) through `_favorite_key` is + left for a later family. The recent-games selection logs at INFO in all + nine. ufc stays on the shared body, dormant: its favourites are fighters, + which its MMA managers match themselves (a follow-up). Fix ported: only a + game with an id can be a duplicate in the selection methods. - **7, other-games rotation.** football advances the rotation window under `_games_lock` (update() and display() both advance it; interleaved, a window of games is skipped) and fixes a favourites-only pool that recomposed diff --git a/mypy-clean.txt b/mypy-clean.txt index aa3ba212..fa11e729 100644 --- a/mypy-clean.txt +++ b/mypy-clean.txt @@ -36,6 +36,7 @@ src/common/sports_card.py src/common/sports_card_wrappers.py src/common/sports_celebration.py src/common/sports_display_rules.py +src/common/sports_favorites.py src/common/sports_fetch.py src/common/sports_font_path.py src/common/sports_game_over.py diff --git a/src/common/README.md b/src/common/README.md index be1d264d..432b0fad 100644 --- a/src/common/README.md +++ b/src/common/README.md @@ -44,6 +44,7 @@ Rules for the package: | [`sports_card_wrappers`](#sports_card_wrappers) | The game renderer's `sports_card` delegations | Yes (scoreboards) | 3.7.0 | | [`sports_celebration`](#sports_celebration) | Draw a scoreboard's score/win celebration | Yes (scoreboards) | 3.7.0 | | [`sports_display_rules`](#sports_display_rules) | Which games a scoreboard shows, for how long, and its scorebug date line | Yes (scoreboards) | 3.8.0 | +| [`sports_favorites`](#sports_favorites) | Which games involve a favourite team, and the favourites-only picks | Yes (scoreboards) | next release | | [`sports_fetch`](#sports_fetch) | Scoreboard season fetch, lookback and live-odds decisions | Yes (scoreboards) | 3.7.0 | | [`sports_font_path`](#sports_font_path) | Find a scoreboard's bundled font whatever the cwd | Yes (scoreboards) | 3.8.0 | | [`sports_game_over`](#sports_game_over) | Whether a game ESPN still lists as live has ended | Yes (scoreboards) | 3.8.1 | @@ -280,6 +281,19 @@ list it before `SportsCoreSharedMixin`) and `SportsGameRulesMixin` `_effective_live_duration()`, the shorter dwell for a non-favourite live game). +### sports_favorites + +[`sports_favorites.py`](sports_favorites.py). Sports family 6, one mixin per +class that carried the methods: `SportsFavoritesMixin` (`SportsCore`: +`_is_favorite_game(game)` and `_favorite_code(value)`), +`SportsUpcomingFavoritesMixin` (`_select_games_for_display`) and +`SportsRecentFavoritesMixin` (`_select_recent_games_for_display`). Each side +of a game is named by `_favorite_key` (from `SportsHelpersMixin`; NRL +overrides it with the team id) and compared with `favorite_teams` stripped and +upper-cased. The selection methods give each favourite up to the per-team +limit, count a game between two favourites for both, and treat only games +with an id as possible duplicates. + ### sports_fetch [`sports_fetch.py`](sports_fetch.py). `SportsFetchMixin`: the `SportsCore` diff --git a/src/common/sports_favorites.py b/src/common/sports_favorites.py new file mode 100644 index 00000000..88e57f4c --- /dev/null +++ b/src/common/sports_favorites.py @@ -0,0 +1,263 @@ +"""Which games involve a favourite team, and which of them to show (sports family 6). + +The scoreboards' favourite matching, reconciled in ledmatrix-plugins +(family 6) from seven ``_is_favorite_game`` bodies, two +``_select_games_for_display`` and three ``_select_recent_games_for_display`` +into one each, and copied here under their existing names: + +- ``SportsFavoritesMixin`` (``SportsCore``): ``_is_favorite_game(game)``, + asked by ``SportsCoreSharedMixin._favorites_first``, the switch-mode + favourite boost (``SportsHelpersMixin._next_switch_index``), the + non-favourite live dwell (``SportsGameRulesMixin._effective_live_duration``) + and the plugins' live rotation; and ``_favorite_code(value)``, the + normalisation both sides of every comparison go through. +- ``SportsUpcomingFavoritesMixin`` (``SportsUpcoming``): + ``_select_games_for_display``, the favourites-only pick of upcoming games. +- ``SportsRecentFavoritesMixin`` (``SportsRecent``): + ``_select_recent_games_for_display``, the same for finished games, most + recent first. + +Each mixin carries only what its class already had, so no manager gains a +method it did not have. + +THE RULE +-------- +Each side of a game is named by ``_favorite_key(game, side)``, the override +point ``SportsHelpersMixin`` (``src.common.sports_helpers``) has carried since +3.5.0: the team abbreviation by default. A sport whose abbreviations are not +unique overrides it -- NRL returns the ESPN team id (and None when the id is +missing), because "NEW" is both Newcastle and New Zealand. That value and every +entry of ``favorite_teams`` are compared as ``_favorite_code`` leaves them: +stripped and upper-cased, a blank or missing value matching nothing. So +" bos" in the config matches BOS. + +The selection methods give each favourite team up to the per-team limit +(``upcoming_games_to_show`` / ``recent_games_to_show``); a game between two +favourites counts for both. Only a game with an id can be a duplicate: two +games without one are two games. + +A new module rather than more methods on ``sports_shared`` or +``sports_helpers``, for the reason ``sports_helpers`` gives: a missing module +fails at load, where the version checks see it; a missing method fails +mid-update. + +WHAT A HOST MUST PROVIDE +------------------------ +Derived by walking every ``self.`` the mixins read; the host-contract +test in ``test/test_sports_favorites.py`` fails if a read is added without +being listed here. + +- ``favorite_teams`` -- the resolved favourites list (``_is_favorite_game``). + The selection methods are handed the list instead. +- ``_favorite_key`` -- ``SportsHelpersMixin`` supplies the default. +- ``_favorite_code`` -- from ``SportsFavoritesMixin``, which the Upcoming and + Recent classes inherit through their ``SportsCore``. +- ``logger`` -- the selection methods log each pick at DEBUG and a summary at + INFO. +- ``upcoming_games_to_show`` (Upcoming) and ``recent_games_to_show`` (Recent) + -- the per-team limits. + +The methods read the game dict's ``id`` and ``start_time_utc`` (selection), +whatever ``_favorite_key`` reads (``home_abbr`` / ``away_abbr`` by default), +and ``home_abbr`` / ``away_abbr`` again for the DEBUG line; any may be missing. + +BASE ORDER +---------- +No other mixin defines these methods, so the position in the bases does not +change which body runs; a method on the plugin's own class still wins. The +mixins have no ``__init__`` and no state. +""" + +import logging +from datetime import datetime, timezone +from typing import Callable, Dict, List, Optional + + +class SportsFavoritesMixin: + """``SportsCore``'s favourite check. See module docstring.""" + + # The host contract, declared for type checking only. + favorite_teams: List[str] + _favorite_key: Callable[[Dict, str], Optional[str]] + + @staticmethod + def _favorite_code(value) -> Optional[str]: + """``value`` as favourites are compared: stripped and upper-cased. + + None for a missing or blank value, which matches nothing. + """ + if value is None: + return None + return str(value).strip().upper() or None + + def _is_favorite_game(self, game: Dict) -> bool: + """Does either side of this game belong to a favourite team? + + ``_favorite_key`` names each side (the abbreviation; nrl overrides it + with the ESPN team id), and both it and ``favorite_teams`` are compared + as ``_favorite_code`` normalises them, so " bos" matches BOS. + """ + favorites = {self._favorite_code(team) for team in self.favorite_teams or ()} + favorites.discard(None) + return any( + self._favorite_code(self._favorite_key(game, side)) in favorites + for side in ("home", "away") + ) + + +class SportsUpcomingFavoritesMixin: + """``SportsUpcoming``'s favourites-only pick. See module docstring.""" + + # The host contract, declared for type checking only. + logger: logging.Logger + upcoming_games_to_show: int + _favorite_key: Callable[[Dict, str], Optional[str]] + _favorite_code: Callable[[object], Optional[str]] + + def _select_games_for_display( + self, processed_games: List[Dict], favorite_teams: List[str] + ) -> List[Dict]: + """ + Single-pass game selection with proper deduplication and counting. + + When a game involves two favorite teams, it counts toward BOTH teams' limits. + This prevents unexpected game counts from the multi-pass algorithm. + Teams are matched as _is_favorite_game matches them. Only a game with + an id can be a duplicate: two games without one are two games. + """ + sorted_games = sorted( + processed_games, + key=lambda g: g.get("start_time_utc") + or datetime.max.replace(tzinfo=timezone.utc), + ) + + if not favorite_teams: + return sorted_games + + selected_games = [] + selected_ids = set() + team_counts: Dict[Optional[str], int] = { + code: 0 for code in map(self._favorite_code, favorite_teams) if code + } + + for game in sorted_games: + game_id = game.get("id") + if game_id is not None and game_id in selected_ids: + continue + + home = self._favorite_code(self._favorite_key(game, "home")) + away = self._favorite_code(self._favorite_key(game, "away")) + + home_fav = home in team_counts + away_fav = away in team_counts + + if not home_fav and not away_fav: + continue + + home_needs = home_fav and team_counts[home] < self.upcoming_games_to_show + away_needs = away_fav and team_counts[away] < self.upcoming_games_to_show + + if home_needs or away_needs: + selected_games.append(game) + if game_id is not None: + selected_ids.add(game_id) + if home_fav: + team_counts[home] += 1 + if away_fav: + team_counts[away] += 1 + + self.logger.debug( + f"Selected game {game.get('away_abbr')}@{game.get('home_abbr')}: " + f"team_counts={team_counts}" + ) + + if all(c >= self.upcoming_games_to_show for c in team_counts.values()): + self.logger.debug("All favorite teams satisfied, stopping selection") + break + + self.logger.info( + f"Selected {len(selected_games)} games for {len(favorite_teams)} " + f"favorite teams: {team_counts}" + ) + return selected_games + + +class SportsRecentFavoritesMixin: + """``SportsRecent``'s favourites-only pick. See module docstring.""" + + # The host contract, declared for type checking only. + logger: logging.Logger + recent_games_to_show: int + _favorite_key: Callable[[Dict, str], Optional[str]] + _favorite_code: Callable[[object], Optional[str]] + + def _select_recent_games_for_display( + self, processed_games: List[Dict], favorite_teams: List[str] + ) -> List[Dict]: + """ + Single-pass game selection for recent games with proper deduplication. + + When a game involves two favorite teams, it counts toward BOTH teams' limits. + Games are sorted by most recent first. + Teams are matched as _is_favorite_game matches them. Only a game with + an id can be a duplicate: two games without one are two games. + """ + sorted_games = sorted( + processed_games, + key=lambda g: g.get("start_time_utc") + or datetime.min.replace(tzinfo=timezone.utc), + reverse=True, + ) + + if not favorite_teams: + return sorted_games + + selected_games = [] + selected_ids = set() + team_counts: Dict[Optional[str], int] = { + code: 0 for code in map(self._favorite_code, favorite_teams) if code + } + + for game in sorted_games: + game_id = game.get("id") + if game_id is not None and game_id in selected_ids: + continue + + home = self._favorite_code(self._favorite_key(game, "home")) + away = self._favorite_code(self._favorite_key(game, "away")) + + home_fav = home in team_counts + away_fav = away in team_counts + + if not home_fav and not away_fav: + continue + + home_needs = home_fav and team_counts[home] < self.recent_games_to_show + away_needs = away_fav and team_counts[away] < self.recent_games_to_show + + if home_needs or away_needs: + selected_games.append(game) + if game_id is not None: + selected_ids.add(game_id) + if home_fav: + team_counts[home] += 1 + if away_fav: + team_counts[away] += 1 + + self.logger.debug( + f"Selected recent game {game.get('away_abbr')}@{game.get('home_abbr')}: " + f"team_counts={team_counts}" + ) + + if all(c >= self.recent_games_to_show for c in team_counts.values()): + self.logger.debug("All favorite teams satisfied, stopping selection") + break + + self.logger.info( + f"Selected {len(selected_games)} recent games for {len(favorite_teams)} " + f"favorite teams: {team_counts}" + ) + return selected_games + + +__all__ = ["SportsFavoritesMixin", "SportsUpcomingFavoritesMixin", "SportsRecentFavoritesMixin"] diff --git a/src/common/sports_helpers.py b/src/common/sports_helpers.py index b78f0ca5..640769a2 100644 --- a/src/common/sports_helpers.py +++ b/src/common/sports_helpers.py @@ -34,8 +34,9 @@ first core release that ships it (see ``CHANGELOG.md``). ``_favorite_key`` is the one method not taken from the plugins: it is the override point from the since-removed ``src/base_classes`` sports core, -carried here so later phases (shared celebrations and game selection) have a -hardware-free home for the seam. No plugin defines it today and nothing in this module calls it. +carried here as the hardware-free home for the seam. ``sports_favorites`` +calls it; nrl overrides it with the ESPN team id. Nothing in this module +calls it. WHAT A HOST MUST PROVIDE ------------------------ @@ -47,8 +48,8 @@ listed here. - ``mode_config`` (dict) and ``logger`` -- ``_setting_int``. ``league`` is read with ``getattr`` for the warning text only. - ``games_list`` and ``current_game_index`` -- ``_next_switch_index``; plus - ``_is_favorite_game`` (called with a game), which stays per-plugin and is - only called when + ``_is_favorite_game`` (called with a game; ``sports_favorites`` has the + shared body), only called when ``favorite_rotation_boost`` is above 1. ``favorite_rotation_boost`` itself defaults to 1 on the mixin. - ``last_game_switch`` -- ``_reset_dwell_on_reentry``, read with ``getattr`` @@ -216,15 +217,12 @@ class SportsHelpersMixin: rather than a branch so core never has to learn the string "nrl":: def _favorite_key(self, game, side): - return str(game.get(f"{side}_id")) + team_id = game.get(f"{side}_id") + return None if team_id is None else str(team_id) - An override that stringifies should note that a missing id becomes the - literal ``"None"``, which would spuriously match a favorites list - containing that string. The default returns ``None`` for a missing - abbreviation, which never matches. - - Carried from the since-removed ``src/base_classes`` sports core for - later phases; nothing in this module calls it yet. + It returns ``None`` for a missing id rather than ``str(None)``, which + would match a favourite typed "None". A ``None`` never matches. + ``sports_favorites`` compares the value stripped and upper-cased. """ return game.get(f"{side}_abbr") diff --git a/test/test_sports_favorites.py b/test/test_sports_favorites.py new file mode 100644 index 00000000..b7e3db70 --- /dev/null +++ b/test/test_sports_favorites.py @@ -0,0 +1,280 @@ +"""src.common.sports_favorites: behaviour, the _favorite_key seam, host contract. + +The cases follow ledmatrix-plugins' ``scripts/test_favourite_matching.py`` +(the tables the family 6 reconcile was checked against), with the favourites +given as each plugin's resolver hands them over: as typed for the +abbreviation sports, as ESPN team ids for an NRL-style host that overrides +``_favorite_key``. +""" + +import ast +import logging +from datetime import datetime, timedelta, timezone +from pathlib import Path + +import pytest + +from src.common import sports_favorites +from src.common.sports_favorites import ( + SportsFavoritesMixin, + SportsRecentFavoritesMixin, + SportsUpcomingFavoritesMixin, +) +from src.common.sports_helpers import SportsHelpersMixin + +LOG = logging.getLogger("test_sports_favorites") + + +def _id_key(self, game, side): + """NRL's override: the ESPN team id, None when it is missing.""" + team_id = game.get(f"{side}_id") + return None if team_id is None else str(team_id) + + +def host(favorites, by_id=False, limit=3): + """A manager stand-in: the three mixins over SportsHelpersMixin's default key.""" + attrs = {"_favorite_key": _id_key} if by_id else {} + cls = type("Host", (SportsUpcomingFavoritesMixin, SportsRecentFavoritesMixin, + SportsFavoritesMixin, SportsHelpersMixin), attrs) + h = cls() + h.logger = LOG + h.favorite_teams = favorites + h.upcoming_games_to_show = h.recent_games_to_show = limit + return h + + +TEAM = {"1": "AAA", "2": "BBB", "3": "CCC", "4": "DDD", "41": "NEW", "42": "NEW"} + + +def match(home, away, **extra): + g = {"home_id": home, "home_abbr": TEAM[home], "away_id": away, "away_abbr": TEAM[away]} + g.update(extra) + return g + + +GAMES = { + "AAA home v BBB": match("1", "2"), + "BBB home v AAA": match("2", "1"), + "CCC v DDD": match("3", "4"), + "Knights (NEW 41) v CCC": match("41", "3"), + "Warriors (NEW 42) v CCC": match("42", "3"), + "AAA v BBB, no ids": {"home_abbr": "AAA", "away_abbr": "BBB"}, + "ids 1 v 2, no abbrs": {"home_id": "1", "away_id": "2"}, + "AAA v BBB, int ids": match("1", "2", home_id=1, away_id=2), + "lower-case abbrs": {"home_abbr": "aaa ", "away_abbr": "bbb"}, + "empty game": {}, +} + +#: label -> favorite_teams as the resolver hands it over. +FAVORITES = { + "none": [], + "AAA": ["AAA"], + "aaa": ["aaa"], + "' AAA '": [" AAA "], + "1": ["1"], + "int 1": [1], + "AAA,CCC": ["AAA", "CCC"], + "NEW": ["NEW"], + "41": ["41"], + "'None'": ["None"], + "blank": ["", " "], +} + +#: (favourites, game) -> answer with the abbreviation key, then the id key. +EXPECTED_IS_FAVORITE = { + "AAA home v BBB": {"AAA": "Y.", "aaa": "Y.", "' AAA '": "Y.", "1": ".Y", "int 1": ".Y", + "AAA,CCC": "Y."}, + "BBB home v AAA": {"AAA": "Y.", "aaa": "Y.", "' AAA '": "Y.", "1": ".Y", "int 1": ".Y", + "AAA,CCC": "Y."}, + "CCC v DDD": {"AAA,CCC": "Y."}, + "Knights (NEW 41) v CCC": {"AAA,CCC": "Y.", "NEW": "Y.", "41": ".Y"}, + "Warriors (NEW 42) v CCC": {"AAA,CCC": "Y.", "NEW": "Y."}, + "AAA v BBB, no ids": {"AAA": "Y.", "aaa": "Y.", "' AAA '": "Y.", "AAA,CCC": "Y."}, + "ids 1 v 2, no abbrs": {"1": ".Y", "int 1": ".Y"}, + "AAA v BBB, int ids": {"AAA": "Y.", "aaa": "Y.", "' AAA '": "Y.", "1": ".Y", + "int 1": ".Y", "AAA,CCC": "Y."}, + "lower-case abbrs": {"AAA": "Y.", "aaa": "Y.", "' AAA '": "Y.", "AAA,CCC": "Y."}, + "empty game": {}, +} + + +@pytest.mark.parametrize("game_label", sorted(GAMES)) +@pytest.mark.parametrize("fav_label", sorted(FAVORITES)) +def test_is_favorite_game(fav_label, game_label): + want = EXPECTED_IS_FAVORITE[game_label].get(fav_label, "..") + got = "".join("Y" if host(FAVORITES[fav_label], by_id)._is_favorite_game(dict(GAMES[game_label])) + else "." for by_id in (False, True)) + assert got == want + + +class TestFavoriteCode: + @pytest.mark.parametrize("value, code", [ + ("bos", "BOS"), (" BOS ", "BOS"), ("BOS", "BOS"), (41, "41"), + ("", None), (" ", None), (None, None), + ]) + def test_normalises(self, value, code): + assert SportsFavoritesMixin._favorite_code(value) == code + + def test_a_missing_id_is_not_the_string_none(self): + """str(None) would match a favourite typed "None"; None matches nothing.""" + h = host(["None"], by_id=True) + assert h._is_favorite_game({"home_abbr": "AAA", "away_abbr": "BBB"}) is False + assert h._is_favorite_game({}) is False + + def test_a_none_favorites_list_matches_nothing(self): + assert host(None)._is_favorite_game(dict(GAMES["AAA home v BBB"])) is False + + +# --------------------------------------------------------------------------- +# Selection. A shuffled slate: two games share id s2, two have no id, s9 has +# no start time. Hours from now; Recent gets the same slate in the past. +# --------------------------------------------------------------------------- + +NOW = datetime(2026, 10, 5, 15, tzinfo=timezone.utc) +SLATE = (("s5", "41", "2", 5), ("s1", "1", "2", 1), ("s3", "4", "3", 3), + ("s2", "3", "1", 2), ("s7", "2", "3", 7), ("s4", "1", "4", 4), + ("s6", "42", "4", 6), ("s2", "1", "4", 8), ("s9", "1", "3", None), + (None, "3", "1", 9), (None, "2", "1", 10)) + + +def slate(recent): + sign = -1 if recent else 1 + games = [] + for gid, home, away, hours in SLATE: + g = match(home, away, id=gid) + if hours is not None: + g["start_time_utc"] = NOW + timedelta(hours=sign * hours) + games.append(g) + return games + + +def pick(favorites, limit, recent, by_id=False): + h = host(favorites, by_id, limit) + method = h._select_recent_games_for_display if recent else h._select_games_for_display + return ",".join(g["id"] or "~" for g in method(slate(recent), favorites)) or "none" + + +ALL = "s1,s2,s3,s4,s5,s6,s7,s2,~,~,s9" + +#: (favourites, per-team limit) -> picked ids, the same for Upcoming and Recent. +EXPECTED_SELECT = { + (("AAA",), 1): "s1", (("AAA",), 2): "s1,s2", (("AAA",), 5): "s1,s2,s4,~,~", + (("aaa",), 5): "s1,s2,s4,~,~", ((" AAA ",), 2): "s1,s2", + (("AAA", "CCC"), 1): "s1,s2", (("AAA", "CCC"), 2): "s1,s2,s3", + (("AAA", "CCC"), 5): "s1,s2,s3,s4,s7,~,~,s9", + (("AAA", "ZZZ"), 2): "s1,s2", (("NEW",), 2): "s5,s6", + (("ZZZ",), 2): "none", ((), 1): ALL, +} + + +@pytest.mark.parametrize("recent", [False, True], ids=["upcoming", "recent"]) +@pytest.mark.parametrize("favorites, limit", sorted(EXPECTED_SELECT)) +def test_select(favorites, limit, recent): + assert pick(list(favorites), limit, recent) == EXPECTED_SELECT[(favorites, limit)] + + +class TestSelectByTeamId: + """The NRL-style host: the key is the team id, so NEW is two teams.""" + + @pytest.mark.parametrize("recent", [False, True]) + def test_one_club_of_a_shared_abbreviation(self, recent): + assert pick(["41"], 2, recent, by_id=True) == "s5" + + @pytest.mark.parametrize("recent", [False, True]) + def test_an_unresolved_abbreviation_matches_nothing(self, recent): + assert pick(["NEW"], 2, recent, by_id=True) == "none" + + def test_ids_select_like_abbreviations(self): + assert pick(["1"], 5, False, by_id=True) == "s1,s2,s4,~,~" + + +class TestSelectionRules: + def test_a_game_between_two_favourites_counts_for_both(self): + h = host(["AAA", "BBB"], limit=1) + picked = h._select_games_for_display(slate(False), ["AAA", "BBB"]) + assert [g["id"] for g in picked] == ["s1"] + + def test_games_without_an_id_are_never_duplicates(self): + games = [match("1", "2"), match("1", "3")] + assert len(host(["AAA"])._select_games_for_display(games, ["AAA"])) == 2 + + def test_a_reused_id_is_a_duplicate(self): + games = [match("1", "2", id="x"), match("1", "3", id="x")] + assert len(host(["AAA"])._select_games_for_display(games, ["AAA"])) == 1 + + def test_upcoming_is_soonest_first_and_recent_newest_first(self): + assert pick(["CCC"], 5, False) == "s2,s3,s7,~,s9" + assert pick(["CCC"], 5, True) == "s2,s3,s7,~,s9" + + def test_the_handed_list_is_used_not_favorite_teams(self): + h = host(["CCC"], limit=1) + assert [g["id"] for g in h._select_games_for_display(slate(False), ["AAA"])] == ["s1"] + + @pytest.mark.parametrize("recent", [False, True]) + def test_the_summary_is_logged_at_info(self, recent, caplog): + with caplog.at_level(logging.INFO, logger=LOG.name): + pick(["AAA"], 1, recent) + name = "_select_recent_games_for_display" if recent else "_select_games_for_display" + assert [r.levelno for r in caplog.records if r.funcName == name + and r.levelno >= logging.INFO] == [logging.INFO] + + +# --------------------------------------------------------------------------- +# Carriers and host contract +# --------------------------------------------------------------------------- + +MIXINS = { + "SportsFavoritesMixin": ["_favorite_code", "_is_favorite_game"], + "SportsUpcomingFavoritesMixin": ["_select_games_for_display"], + "SportsRecentFavoritesMixin": ["_select_recent_games_for_display"], +} + + +def _classes(): + tree = ast.parse(Path(sports_favorites.__file__).read_text(encoding="utf-8")) + return {n.name: n for n in tree.body if isinstance(n, ast.ClassDef)} + + +def _self_reads(cls): + return {node.attr for node in ast.walk(cls) + if isinstance(node, ast.Attribute) and isinstance(node.ctx, ast.Load) + and isinstance(node.value, ast.Name) and node.value.id == "self"} + + +class TestHostContract: + def test_each_mixin_carries_only_its_class_methods(self): + """So adopting one gives no manager a method it did not have.""" + for name, methods in MIXINS.items(): + mixin = getattr(sports_favorites, name) + assert sorted(n for n in vars(mixin) if not n.startswith("__")) == methods + + def test_every_host_read_is_documented(self): + reads = set().union(*(_self_reads(c) for c in _classes().values())) + undocumented = sorted(n for n in reads if f"``{n}``" not in sports_favorites.__doc__) + assert undocumented == [], f"read but not in the host contract: {undocumented}" + + def test_the_key_comes_from_sports_helpers(self): + """The seam stays where 3.5.0 put it; this module only calls it.""" + assert "_favorite_key" in vars(SportsHelpersMixin) + assert all("_favorite_key" not in vars(getattr(sports_favorites, n)) for n in MIXINS) + + def test_no_other_shared_mixin_defines_these(self): + from src.common import sports_display_rules, sports_shared + others = [sports_shared.SportsCoreSharedMixin, sports_shared.SportsRecentSharedMixin, + sports_shared.SportsLiveSharedMixin, SportsHelpersMixin, + sports_display_rules.SportsGameRulesMixin] + for methods in MIXINS.values(): + for name in methods: + assert not any(name in vars(o) for o in others), name + + def test_the_shared_callers_reach_it(self): + """_favorites_first and the live dwell ask _is_favorite_game; one body answers.""" + from src.common.sports_display_rules import SportsGameRulesMixin + + class Host(SportsGameRulesMixin, SportsFavoritesMixin, SportsHelpersMixin): + favorite_teams = ["aaa"] + game_display_duration = 15 + non_favorite_live_game_duration = 5 + + assert Host()._effective_live_duration(dict(GAMES["AAA home v BBB"])) == 15 + assert Host()._effective_live_duration(dict(GAMES["CCC v DDD"])) == 5 diff --git a/test/test_sports_favorites_parity.py b/test/test_sports_favorites_parity.py new file mode 100644 index 00000000..407b7172 --- /dev/null +++ b/test/test_sports_favorites_parity.py @@ -0,0 +1,135 @@ +"""sports_favorites still matches every plugin copy, and only nrl overrides the key. + +``src.common.sports_favorites`` was copied from the scoreboards once family 6 +had made each method one body in all nine: ``SportsCore._favorite_code`` and +``_is_favorite_game``, ``SportsUpcoming._select_games_for_display`` and +``SportsRecent._select_recent_games_for_display``. The plugins delete their +copies once they floor on the release that ships this module. Until each has, a +copy that changes on its own is a fix one side has and the other lacks. + +Point LEDMATRIX_PLUGINS at a ledmatrix-plugins checkout and each method is +compared with every plugin copy using ``scripts/sports_drift_report.py``'s own +normalisation (the AST with docstrings and annotations dropped), plus the +decorators. A copy that is gone counts as adopted when the plugin's +``sports.py`` names the module. The owner's decision that only nrl overrides +``_favorite_key`` (with the team id) is checked too; that override stays in +the plugin after adoption. Without the variable this skips: core CI has no +plugins checkout. +""" + +import ast +import importlib.util +import os +from pathlib import Path + +import pytest + +from src.common import sports_favorites + +REPO = Path(__file__).resolve().parents[1] +SPORTS = ("afl", "baseball", "basketball", "football", "hockey", "lacrosse", + "nrl", "soccer", "ufc") + +#: plugin class -> (our mixin, the methods it carries) +CARRIERS = { + "SportsCore": ("SportsFavoritesMixin", ("_favorite_code", "_is_favorite_game")), + "SportsUpcoming": ("SportsUpcomingFavoritesMixin", ("_select_games_for_display",)), + "SportsRecent": ("SportsRecentFavoritesMixin", ("_select_recent_games_for_display",)), +} + +#: The owner's decision (docs/SPORTS_UNIFICATION.md, family 6): the sports +#: that name a team by something other than its abbreviation. +OVERRIDES_FAVORITE_KEY = {"nrl"} + + +def _drift_report(): + """scripts/sports_drift_report.py, loaded by path (scripts/ is no package).""" + spec = importlib.util.spec_from_file_location( + "sports_drift_report", REPO / "scripts" / "sports_drift_report.py") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +DRIFT = _drift_report() + + +def _plugins_root(): + root = DRIFT.resolve_plugins_dir(os.environ.get("LEDMATRIX_PLUGINS")) + if root is None: + pytest.skip("set LEDMATRIX_PLUGINS to a ledmatrix-plugins checkout to " + "compare this module against the plugin copies") + return root + + +def _class(tree, name): + return next(n for n in tree.body if isinstance(n, ast.ClassDef) and n.name == name) + + +def _method(cls, name): + return next((n for n in cls.body + if isinstance(n, ast.FunctionDef) and n.name == name), None) + + +def _fingerprint(node): + return (DRIFT._digest(node, DRIFT._Canonical()), + tuple(ast.unparse(d) for d in node.decorator_list)) + + +def _ours(mixin): + tree = ast.parse(Path(sports_favorites.__file__).read_text(encoding="utf-8")) + return _class(tree, mixin) + + +def _plugin_tree(root, sport): + source = (root / f"{sport}-scoreboard" / "sports.py").read_text(encoding="utf-8") + return source, ast.parse(source) + + +CASES = [(sport, cls, name) for sport in SPORTS + for cls, (_, names) in CARRIERS.items() for name in names] + + +@pytest.mark.parametrize("sport, cls, name", CASES) +def test_every_remaining_plugin_copy_matches(sport, cls, name): + source, tree = _plugin_tree(_plugins_root(), sport) + copy = _method(_class(tree, cls), name) + if copy is None: + assert sports_favorites.__name__ in source, ( + f"{sport}: no {name} on {cls} and no {sports_favorites.__name__} import") + else: + ours = _method(_ours(CARRIERS[cls][0]), name) + assert _fingerprint(copy) == _fingerprint(ours), ( + f"{cls}.{name} in {sport} differs from sports_favorites. " + f"Port the change to both, or stop treating it as shared.") + + +@pytest.mark.parametrize("sport", SPORTS) +def test_no_other_plugin_class_carries_a_copy(sport): + """A copy on another class (afl's old SportsUpcoming._is_favorite_game) would shadow the shared one.""" + _, tree = _plugin_tree(_plugins_root(), sport) + shared = {name: cls for cls, (_, names) in CARRIERS.items() for name in names} + strays = [f"{node.name}.{name}" for node in tree.body if isinstance(node, ast.ClassDef) + for name, home in shared.items() + if node.name != home and _method(node, name) is not None] + assert strays == [] + + +def test_only_the_decided_sports_override_the_key(): + root = _plugins_root() + overriding = {sport for sport in SPORTS + if any(_method(node, "_favorite_key") is not None + for node in _plugin_tree(root, sport)[1].body + if isinstance(node, ast.ClassDef))} + assert overriding == OVERRIDES_FAVORITE_KEY + + +def test_the_drift_report_still_calls_them_identical(): + root = _plugins_root() + families = DRIFT.build(root, ("sports.py",)) + rows = {(r["file"], r["family"]): r + for r in (DRIFT.summarise(k, v) for k, v in families.items())} + for _, names in CARRIERS.values(): + for name in names: + row = rows.get(("sports.py", name)) + assert row is None or row["worst_class_variants"] == 1, name From e40bc47d28d484e89d7ba49f023c166aeb945ed3 Mon Sep 17 00:00:00 2001 From: Chuck <33324927+ChuckBuilds@users.noreply.github.com> Date: Mon, 5 Oct 2026 12:50:05 -0400 Subject: [PATCH 8/8] chore: prepare the 3.8.2 release (#776) Bumps src.__version__ to 3.8.2 and turns Unreleased into ## 3.8.2: the display's malloc arena cap and between-screen malloc_trim (#774), and src.common.sports_favorites (#775, sports family 6), which the scoreboards adopt by flooring on 3.8.2. src/common/README.md and docs/SPORTS_UNIFICATION.md say 3.8.2 for it; the SPORTS_UNIFICATION module table also said "next release" for the four stage 4 modules, which shipped in 3.8.0. Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 10 ++++++++-- docs/SPORTS_UNIFICATION.md | 10 +++++----- src/__init__.py | 2 +- src/common/README.md | 2 +- 4 files changed, 15 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 90e0e87e..7f729e9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,12 @@ accepts both, but the store flags the old spelling as deprecated ## Unreleased +## 3.8.2 + +The display hands freed memory back to the OS (#774), and sports consolidation +family 6: `src.common.sports_favorites`, which the scoreboards adopt by +flooring on 3.8.2 (#775). + ### The display hands freed memory back to the OS The display process's resident memory climbed in steps for hours while the @@ -52,8 +58,8 @@ and the 20-minute frame soaks came out at 0.147-0.165% late against main's favourites-only picks). Each side of a game is named by the 3.5.0 `_favorite_key` seam and compared with `favorite_teams` stripped and upper-cased; nrl overrides the key with the ESPN team id. Only a game with an - id can be a duplicate. A plugin may inherit the mixins once it floors on the - release that ships this module, and deletes its copies then. + id can be a duplicate. A plugin may inherit the mixins once it floors on + 3.8.2, and deletes its copies then. (#775) ## 3.8.1 diff --git a/docs/SPORTS_UNIFICATION.md b/docs/SPORTS_UNIFICATION.md index f30e8a4c..4363e43d 100644 --- a/docs/SPORTS_UNIFICATION.md +++ b/docs/SPORTS_UNIFICATION.md @@ -87,12 +87,12 @@ more. Shared sports code lives in `src/common`: | `sports_celebration.py` | 3.7.0 | `SportsCelebrationMixin` — draws the score/win takeover; colour helpers | | `sports_fetch.py` | 3.7.0 | `SportsFetchMixin` — season fetch, live lookback and live-odds decisions | | `sports_card_wrappers.py` | 3.7.0 | `SportsCardWrappersMixin` — the game renderer's `sports_card` delegations | -| `sports_plugin_host.py` | next release | `SportsPluginHostMixin` — the plugin class's (`manager.py`) identical helpers: Vegas weight, off-thread switch refresh | -| `sports_live_scroll.py` | next release | `SportsLiveScrollMixin` — rebuild a live scroll strip mid-cycle, keeping the marquee's place | -| `sports_display_rules.py` | next release | `SportsCardOptionsMixin`, `SportsGameRulesMixin` — scorebug date options, the no-favourites filter, non-favourite live dwell | -| `sports_font_path.py` | next release | `resolve_font_path` — what the plugins' `_resolve_font_path` copies return | +| `sports_plugin_host.py` | 3.8.0 | `SportsPluginHostMixin` — the plugin class's (`manager.py`) identical helpers: Vegas weight, off-thread switch refresh | +| `sports_live_scroll.py` | 3.8.0 | `SportsLiveScrollMixin` — rebuild a live scroll strip mid-cycle, keeping the marquee's place | +| `sports_display_rules.py` | 3.8.0 | `SportsCardOptionsMixin`, `SportsGameRulesMixin` — scorebug date options, the no-favourites filter, non-favourite live dwell | +| `sports_font_path.py` | 3.8.0 | `resolve_font_path` — what the plugins' `_resolve_font_path` copies return | | `sports_game_over.py` | 3.8.1 | `SportsGameOverMixin` — `_is_game_really_over`, with the `FINAL_PERIOD` seam (family 5) | -| `sports_favorites.py` | next release | `SportsFavoritesMixin`, `SportsUpcomingFavoritesMixin`, `SportsRecentFavoritesMixin` — `_is_favorite_game` and the favourites-only picks, on the `_favorite_key` seam (family 6) | +| `sports_favorites.py` | 3.8.2 | `SportsFavoritesMixin`, `SportsUpcomingFavoritesMixin`, `SportsRecentFavoritesMixin` — `_is_favorite_game` and the favourites-only picks, on the `_favorite_key` seam (family 6) | Each is described in [src/common/README.md](../src/common/README.md). diff --git a/src/__init__.py b/src/__init__.py index 9bf9335e..add0e258 100644 --- a/src/__init__.py +++ b/src/__init__.py @@ -4,5 +4,5 @@ LEDMatrix Display System Core source package for the LED Matrix Display project. """ -__version__ = "3.8.1" +__version__ = "3.8.2" diff --git a/src/common/README.md b/src/common/README.md index 432b0fad..66140ede 100644 --- a/src/common/README.md +++ b/src/common/README.md @@ -44,7 +44,7 @@ Rules for the package: | [`sports_card_wrappers`](#sports_card_wrappers) | The game renderer's `sports_card` delegations | Yes (scoreboards) | 3.7.0 | | [`sports_celebration`](#sports_celebration) | Draw a scoreboard's score/win celebration | Yes (scoreboards) | 3.7.0 | | [`sports_display_rules`](#sports_display_rules) | Which games a scoreboard shows, for how long, and its scorebug date line | Yes (scoreboards) | 3.8.0 | -| [`sports_favorites`](#sports_favorites) | Which games involve a favourite team, and the favourites-only picks | Yes (scoreboards) | next release | +| [`sports_favorites`](#sports_favorites) | Which games involve a favourite team, and the favourites-only picks | Yes (scoreboards) | 3.8.2 | | [`sports_fetch`](#sports_fetch) | Scoreboard season fetch, lookback and live-odds decisions | Yes (scoreboards) | 3.7.0 | | [`sports_font_path`](#sports_font_path) | Find a scoreboard's bundled font whatever the cwd | Yes (scoreboards) | 3.8.0 | | [`sports_game_over`](#sports_game_over) | Whether a game ESPN still lists as live has ended | Yes (scoreboards) | 3.8.1 |