fix(sports): finish the has_live_content log throttle for three plugins - #308
fix(sports): finish the has_live_content log throttle for three plugins#308ChuckBuilds wants to merge 1 commit into
Conversation
#241 fixed this for baseball and football and left hockey, basketball and lacrosse carrying the original pattern: should_log = result or (current_time - self._last_live_content_false_log >= self._live_content_log_interval) "Always log True immediately (important)" is the bug. has_live_content() is called once per frame on the display path, not once per update, so a True answer logs at frame rate for as long as the games stay live. The False answer was the only one throttled. In hockey and basketball both branches then logged the same message, so the if/else differed only in whether it updated the timestamp. When this was measured on the baseball copy it was 13,871 lines a minute, 98% of the journal, and every one of those lines is a journal write to an SD card. These three are dormant in August, which is presumably why they were left: NHL resumes in October, NCAA basketball in November, lacrosse in February. Replaced with the shape baseball already uses -- log when the answer changes, and otherwise once per interval so a steady state stays visible. The per-league booleans form the state tuple, so a game starting or ending still logs immediately, which is the part worth keeping. UFC was checked and is unaffected: its guard is `should_log and not ufc_live`, so the True path logs nothing. Soccer declares the timestamp field but never reads it, and logs nothing unthrottled; left alone rather than churn a version for a dead field. Safety harness: 56 pass, 0 fail across the three. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
|
Superseded by #315, which combines this with the other five sports PRs so each plugin gets one version bump instead of up to four competing ones. Every change from this PR is verified present on that branch; the branch here is untouched if you want to compare. |
…games (#315) * fix(sports): stop odds overprinting the time on a narrow card Reported as the green odds text being wrong on football upcoming games in Vegas mode. The odds themselves were fine -- the spread value and the favoured side were checked against ten live ESPN games and were right every time. The problem is layout. Both odds labels are anchored to the edges of the top row, and the card centres its time text on that same row. On a full-width panel there is room for all three. On a Vegas game card there is not: these plugins pin the card to 128px regardless of panel width (game_card_width: 128), and at that size O/U: 60.5 occupies x 0.. 54 8:00 PM occupies x 36.. 92 -38.5 occupies x 98..128 so the O/U runs 18px into the time and both become unreadable. Reproduced by rendering the same upcoming game at 128px and at 512px with a live rig's own config; the full-width render was clean, the card was not. Why it survived this long: whether it bites depends on the configured font. With the default 4x6 detail font "O/U: 60.5" is 30px against a 32px budget and fits by two pixels. The rig configures PressStart2P, which makes it 54px. Each renderer now budgets a side against the widest time string the centre can hold, measured in the font it will actually use rather than hard-coded, and drops the O/U -- the longer label and the less useful number -- when it will not fit. The right-anchored label also moves in a pixel so its outline stroke is not clipped by the edge. Full-width output is byte-identical in green-pixel terms: 150px before and after. Six plugins carried it, each with a diverged copy of the routine, so each was patched to its own shape rather than by one sweep. baseball-scoreboard is deliberately untouched. It already solves this, and better: it measures the card's own top-row text and steps the odds down a row when they would collide, keeping both labels. Its answer leaves them in the same columns, so the column-based check here would fail a correct implementation; the test skips it and says why. scripts/test_odds_centre_collision.py renders each plugin's odds at both widths and asserts nothing green lands in the centre band, that the card still shows the spread, and that a full panel still shows both labels. Mutation-checked: removing a budget, or making it always pass, fails the check by name. Two false alarms found while writing that harness, both mine: dispatching on parameter count rather than name fed the width into baseball's `top_span` and drew nothing at all, and an earlier render with no logo cache took the logo-fallback early return and drew no odds either. Both looked like defects and were not. * test(sports): load the odds renderers in-process instead of per-plugin subprocesses Codacy failed the branch with five findings, all in the new test harness. I pulled the annotations off the check run rather than guessing: 'inspect' imported but unused (F401) 'logging' imported but unused (F401) Consider possible security implications associated with the subprocess module Detected subprocess function 'run' without a static string subprocess call - check for execution of untrusted input The subprocess existed to dodge a real constraint: every plugin ships its own game_renderer.py, so importing several in one process hands the second plugin the first one's cached module. But the repo already solves that in-process -- scripts/test_schedule_window_plumbing.py drops anything previously imported out of a plugin directory and keeps only the current plugin on sys.path. Using the same approach removes all five findings rather than suppressing three of them, and drops the two imports that only existed to build the subprocess call. It is also faster: one interpreter instead of one per plugin. Same assertions, same result, and the mutation check still holds -- removing a plugin's O/U budget fails two checks. * fix(sports): finish the has_live_content log throttle for three plugins #241 fixed this for baseball and football and left hockey, basketball and lacrosse carrying the original pattern: should_log = result or (current_time - self._last_live_content_false_log >= self._live_content_log_interval) "Always log True immediately (important)" is the bug. has_live_content() is called once per frame on the display path, not once per update, so a True answer logs at frame rate for as long as the games stay live. The False answer was the only one throttled. In hockey and basketball both branches then logged the same message, so the if/else differed only in whether it updated the timestamp. When this was measured on the baseball copy it was 13,871 lines a minute, 98% of the journal, and every one of those lines is a journal write to an SD card. These three are dormant in August, which is presumably why they were left: NHL resumes in October, NCAA basketball in November, lacrosse in February. Replaced with the shape baseball already uses -- log when the answer changes, and otherwise once per interval so a steady state stays visible. The per-league booleans form the state tuple, so a game starting or ending still logs immediately, which is the part worth keeping. UFC was checked and is unaffected: its guard is `should_log and not ufc_live`, so the True path logs nothing. Soccer declares the timestamp field but never reads it, and logs nothing unthrottled; left alone rather than churn a version for a dead field. Safety harness: 56 pass, 0 fail across the three. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW * perf(sports): drop get_cycle_duration's diagnostics to debug level get_cycle_duration() opens with an unconditional INFO line naming its arguments, and five sports plugins carry between eight and twelve more through the body -- "returning None", "extracted mode_type=", and so on. All of it is diagnostic: it describes the function's own control flow, not anything that happened to the display. The core calls this from the main display loop. In display_controller.run(), once a plugin has rendered content, the else branch reads the base duration and then, if dynamic duration is enabled for the mode, queries the plugin for its cycle duration. That loop is paced at frame rate, so every one of those INFO lines lands at frame rate for as long as the mode is on screen. It is gated behind dynamic_duration being enabled, which is why it is not visible on the rig I can measure -- that config has the feature off, and the journal shows zero such lines in six hours. It is not an obscure setting though; football ships DYNAMIC_DURATION.md telling users how to turn it on. Same shape as the has_live_content flood fixed in #241: a per-frame caller meeting logging written as if it ran once. Demoted rather than throttled, because unlike the live-content summary none of this is worth an operator's attention even once -- it is what you turn debug on to see. 48 lines across five plugins. No behaviour change. Safety harness: 104 pass, 0 fail across all five. Versions land above the ones in #308 so the two merge in ascending order. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW * feat(football): full-height logos and readable odds on tall scroll cards Two things looked wrong on a 64-tall panel in scroll and Vegas mode: the team logos were visibly smaller than the same game on the regular display, and the odds were a 6px afterthought tucked into the corners. Both come from sizes chosen for a 32-tall panel and never revisited. Scroll cards were a flat 128px wide whatever the panel was. GameRenderer gives each logo (card width - centre gap) / 2, capped at the card height, so on a 64-tall panel that came to (128 - 36) / 2 = 46px -- logos stuck at 71% of the height they had room for, while the regular 512px card rendered the same logo at a full 64px. The default card width is now two full-height logos plus the gap, which makes the height the binding constraint again, as the renderer's cap already assumes. A 32-tall panel computes 104 and keeps the 128 it always had, so only tall panels move. The odds are drawn in the detail font, pinned at 6px. That font now scales with panel height: 6px at 32 tall, 10px at 64. The cap is 1.75x rather than the 2x the ratio would give, because at 12px the over/under no longer fits beside the centre text and the collision guard drops it -- the odds would have got bigger by losing half of themselves. 10px was the largest size measured to still render both numbers. Scaling that font exposed a hardcoded `date_y = display_height - 7`, which fitted a 6px glyph and clipped a 10px one. The date now sits on the bottom edge by its own measured height. Also fixed the logo cache, which never hit: entries are stored under "<abbr>@<slot>x<height>" but the lookup tested the bare abbreviation, so every card re-opened and re-resized both PNGs -- once per game per rebuild on the scroll path. baseball, basketball and lacrosse already guard on the scoped key; football and NRL did not (NRL still does not, and wants the same fix). A user-configured font size is untouched throughout; only defaults move. Safety harness: 24 pass, 0 fail across all eight sizes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW * perf(sports): make the logo cache actually hit Five scoreboards filled a logo cache they could never read from. The key is scoped by slot size -- "<abbr>@<slot>x<height>" -- because one cache dict is shared by renderers built for different card widths, but the membership test used the unscoped name: if team_abbrev in self._logo_cache: # never present return self._logo_cache[self._logo_cache_key(...)] # what writes use So the guard never matched, and every card re-opened and re-resized both source PNGs. Measured on a Pi, five loads of one logo: main: 5 calls -> 5 image decodes fixed: 5 calls -> 1 image decode What that costs depends entirely on the asset, which is why this stayed invisible. A 64x64 logo resizes in 0.7ms and nobody notices. But some shipped logos are far larger -- football's GB.png is 4096x4096 -- and LANCZOS-scaling that down to 64px takes roughly 800ms on a Pi, paid once per game per scroll rebuild rather than once ever. afl, nrl and soccer tested the bare abbreviation. basketball and lacrosse tested an unscoped "<league>:<abbr>" that their writes then scoped. lacrosse also had a "backward compatibility" fallback with the same defect. baseball was already correct. Rather than five copies of a unit test, scripts/test_logo_cache_key_consistency.py asserts the invariant across every renderer: if a plugin scopes its writes, its guards must use the same key. It traces locals, so `k = self._logo_cache_key(x)` followed by `if k in self._logo_cache` reads as correct. It flags football, which is fixed in the separate PR that this one deliberately does not touch. Safety harness: 112 pass, 0 fail across the five. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW * fix(sports): stop live games going stale for an hour in Vegas A Rays game that had finished was still drawn as live on a running rig. The Vegas strip had not been rebuilt for 45 minutes, while three other games were genuinely live and changing every pitch. Vegas rebuilds a plugin's cards when the core reports that plugin's update() finished. The core schedules update() from the plugin's update_interval, and for baseball that resolved to 3600 -- so its cards refresh hourly. Measured on the rig: 863 Vegas update ticks in an hour, of which exactly one mentioned baseball, at 14:41 and then 15:40. The per-mode intervals cannot help. mlb.live_update_interval is 30, but the live manager only runs inside update(), so a 30-second gate behind an hourly door opens hourly. has_live_content() cannot help either -- it is gated on live_priority, which is off in that config, so the plugin never reports live content for anything upstream to react to. _get_plugin_update_interval reads the manifest first and only then the config, so a manifest value fixes existing installs without anyone editing their settings. hockey, lacrosse and ufc already ship update_interval 60 for exactly this reason; baseball, football and basketball were missing it and inherited their schema default of 3600. This is not more API traffic. The sub-managers gate their own fetches -- recent and upcoming at 3600, live at its own interval -- so calling update() each minute lets the live path run at the cadence it was configured for and leaves the schedule fetches exactly where they were. That is the design the per-mode intervals were written for; an hourly plugin-level gate defeats it. Safety harness: 72 pass, 0 fail across the three. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW * fix(sports): stop the over/under printing through the status text Reported on a 128x32 rig: the O/U sat in the top centre rather than a corner. It is drawn there deliberately, by a branch nobody hits often: if favored_side == "home": ou_x = 0 # left elif favored_side == "away": ou_x = display_width - ou_width # right else: ou_x = (display_width - ou_width) // 2 # centre The centre of that row is where the card puts its status text, so with no favourite the O/U and "Final" render through each other. Reproduced at 128x32: "F O/U:47.5 l". There is no favourite whenever the spread is a pick'em 0, or absent from the odds payload, or not a number -- and ESPN's pointSpread carries no "value" key, so the top-level spread is the primary source rather than a fallback. Anchored to the same left edge the home-favoured case uses. That is also what the side budget added in #302 assumes: it measures the label against the space beside the centre, which only holds if the label is against an edge. Also guarded the negation that builds the away spread. It ran before the isinstance checks below it, so a spread arriving as a string raised TypeError, the handler swallowed it, and the whole odds block silently disappeared. Now a non-numeric spread means no favourite -- the O/U still draws. Six plugins had the centring, seven had the unguarded negation. Safety harness: 160 pass, 0 fail across the seven. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW * fix(odds-ticker): stop an unconfigured ticker inheriting the legacy 20fps The scroll settings resolve through a precedence chain ending in a legacy branch, and that branch means two different things: "this config predates display_options" and "nothing is configured at all". They want different defaults and were getting the same one. The legacy pair -- 2px per frame at 0.05s, 20fps -- is right for a config written before display_options existed, because it keeps that rig scrolling exactly as it did. It is wrong for a plugin with no scroll settings, which inherited a 20fps ticker while the schema's own display_options defaults describe 1px at 0.02s, 50fps. Same net speed either way, roughly 40 vs 50 px/s; the difference is 2px steps twenty times a second against 1px steps fifty times a second, which is the whole of how smooth it looks. That is not a hypothetical shape. A running rig's odds-ticker config is {"enabled": true} and nothing else -- the schema's defaults were never written into it -- so it takes this branch every time. Legacy defaults now apply only when a legacy key is actually present. Verified across all four config shapes: unconfigured 50fps, legacy flat 20fps unchanged, legacy pixels-per-second 20fps unchanged, display_options 50fps. Safety harness: 8 pass, 0 fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW * fix(soccer): stop custom leagues fetching twelve times as often as built-ins A custom league polled ESPN every 300 seconds while every built-in league polled every 3600. Nothing in the UI showed that or could change it. The built-in path reads update_interval_seconds from the league's config, which the schema declares at 3600 in each of the eleven places it appears, so a configured rig carries an explicit 3600 and gets it. The custom-league path reads the same key from the custom league's own dict with a fallback of 300 -- and custom_leagues declared no such key, so it was never populated, the fallback always won, and the setting could not be seen or edited. On a running rig that is one custom league at 288 fetches a day against 24 for each built-in one. Both manager fallbacks are now 3600, matching the schema and the built-in leagues, and custom_leagues declares update_interval_seconds and live_update_interval so the UI offers them. A user who wants a faster custom league can now ask for one instead of getting it by accident. Built-in leagues are unaffected: all ten already carry an explicit 3600. Safety harness: 24 pass, 0 fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01STMbQE4YctTacQXfbYqKuW * Revert "fix(odds-ticker): stop an unconfigured ticker inheriting the legacy 20fps" This reverts commit ebc3682. --------- Co-authored-by: Claude <noreply@anthropic.com>
…frame (#474) has_live_content() runs on the display path -- once per *frame* in Vegas mode. UFC guarded only the False branch: `should_log and not ufc_live` sat at the bottom of the function, but a second INFO line lived inside the `if live_games:` block above it with no guard at all. Whenever a card was live that call fired on every frame, roughly 50 lines a second on a rig measured at 50 fps. Because it sat outside the guard, every earlier throttle fix missed it -- baseball 1.20.4, football, and #308 for hockey/basketball/lacrosse all inspected the guard rather than the whole function body. It has been there since the plugin landed in #24. Both branches now share one throttle in baseball-scoreboard's shape: log when the answer changes, and re-log an unchanged answer once per 60s so a steady state stays visible. test_live_content_log_throttle.py is ported from baseball's, plus a case for the exact state that triggered it -- a live card with favorites configured and none of them fighting, where ufc_live is False but live_games is not empty. Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 Co-authored-by: Claude <noreply@anthropic.com>
…step (#478) * fix(ufc): throttle has_live_content(), which logged once per display frame has_live_content() runs on the display path -- once per *frame* in Vegas mode. UFC guarded only the False branch: `should_log and not ufc_live` sat at the bottom of the function, but a second INFO line lived inside the `if live_games:` block above it with no guard at all. Whenever a card was live that call fired on every frame, roughly 50 lines a second on a rig measured at 50 fps. Because it sat outside the guard, every earlier throttle fix missed it -- baseball 1.20.4, football, and #308 for hockey/basketball/lacrosse all inspected the guard rather than the whole function body. It has been there since the plugin landed in #24. Both branches now share one throttle in baseball-scoreboard's shape: log when the answer changes, and re-log an unchanged answer once per 60s so a steady state stays visible. test_live_content_log_throttle.py is ported from baseball's, plus a case for the exact state that triggered it -- a live card with favorites configured and none of them fighting, where ufc_live is False but live_games is not empty. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 * ci(sports): gate the copy-forward rule that keeps the scoreboards in step The scoreboards ship copies of sports.py, manager.py, game_renderer.py and scroll_display.py rather than sharing them, which is deliberate (CLAUDE.md non-negotiable #7). The rule that makes it safe -- port a fix to its siblings in the same PR -- was enforced only by whoever remembered. It was missed repeatedly. The has_live_content() throttle went into baseball (1.20.4), then football, then hockey/basketball/lacrosse (#308), and ufc was still unfixed months later, logging once per display frame during a live card. Two checks, both baselined against the current tree so they start green and fail only on something new. 1. Shape drift. A function whose copies all AGREE today must not start disagreeing. Bodies compare as normalised ASTs: docstrings dropped, numeric constants collapsed, and sport tokens folded so mlb_live and nfl_live are the same shape. Folding handles CamelCase as well as snake_case, which is load-bearing -- each plugin names its manager class after its own sport, so without it nothing in manager.py is ever compared to its siblings, and has_live_content() lives in manager.py. 46 functions are unanimous across five or more lineages; those are what this protects. 140 already differ and are recorded and ignored: the goal is not uniformity, it is that a shared fix reaches every sibling. 2. Unthrottled display-path logging. An info/warning call in has_live_content(), display() or get_vegas_content() with no throttle guard above it. Shape comparison cannot catch this: has_live_content() legitimately differs between sports -- one league versus three -- so it is already divergent everywhere and similarity has nothing to compare. This is a property check instead, and it is the one that catches the ufc bug. Verified end to end rather than assumed: - clean tree -> exit 0 - revert the ufc fix -> exit 1, naming manager.py:863 - edit one lineage's copy of _coerce_rgb (shared by 8) -> exit 1, naming hockey as the odd one out test_check_sports_drift.py is the gate's own regression suite, following the pattern module-collisions.yml already uses. This check reports by absence, so "looked and found nothing" and "did not really look" both print OK -- the first version of it recursed into class bodies with a bare walk() instead of `yield from`, inspected 16 functions instead of 386, and reported a clean result on code it never opened. The suite asserts it still detects, and that it is still reading a plausible amount of code. Depends on the ufc-scoreboard throttle fix: the baseline is generated with that fix in place, so reintroducing the bug fails the gate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014RRtqXDCnvnY6EQwhT5CV9 --------- Co-authored-by: Claude <noreply@anthropic.com>
#241 fixed this for baseball and football. Hockey, basketball and lacrosse still carry the original:
Why
result oris the bughas_live_content()runs once per frame on the display path, not once per update. So aTrueanswer — which bypasses the throttle by design, per the comment "Always log True immediately (important)" — logs at frame rate for as long as the games stay live. Only theFalseanswer was ever throttled.In hockey and basketball both branches then log the same message; the
if/elsediffers only in whether it updates the timestamp.When this was measured on the baseball copy it came to 13,871 lines a minute, 98% of the journal — and every one is a journal write to an SD card, on hardware that has already destroyed two.
Why it hasn't been noticed
All three are dormant in August, which is presumably why they were left behind:
It fires the moment games start.
The fix
The shape baseball already uses: log when the answer changes, and otherwise once per interval so a steady state stays visible. The per-league booleans form the state tuple, so a game starting or ending still logs immediately — that transition is the part actually worth having.
Checked and deliberately not touched
should_log and not ufc_live, so the True path logs nothing; theufc_live or ...in the condition is neutralised._last_live_content_false_logand never reads it, and logs nothing unthrottled. Left alone rather than churn a version for a dead field.Verification
Safety harness: 56 pass, 0 fail, 0 errors across the three plugins at all sizes.
Versions: hockey 1.13.2, basketball 1.16.3, lacrosse 1.12.2.