feat(football): make the live screen renderable offline - #267
Conversation
The season starts in weeks and live is the screen that carries it, yet it was the one screen the safety harness never rendered. The fixture said why: "_fetch_todays_games() is a direct network call with no cache read and no test_mode passthrough in _adapt_config_for_manager, so the live screen can never be fed from mock data." Both halves of that turned out to be bugs rather than gaps. _adapt_config_for_manager() never passed test_mode into the per-manager config, so SportsLive.test_mode could not be set from config at all -- which made the fully seeded simulated game already sitting in NFLLiveManager.__init__ unreachable, along with everything behind `if self.test_mode`. SportsLive.update() then fetched unconditionally, so even with the flag set the seeded game was overwritten on the first tick. _test_mode_update() was defined and never called: dead code. baseball-scoreboard had already fixed exactly this, so the short-circuit is ported from there rather than invented. With both fixed the harness renders nfl_live at every panel size, from 16 renders to 24, none of them blank -- the simulated card draws the score, the quarter and clock, down and distance, possession and timeouts, which are the parts only this mode has. No behaviour change for anyone running it: test_mode defaults to False on both paths, so live fetches exactly as before. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
|
Warning Review limit reached
Next review available in: 56 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe football scoreboard now propagates league ChangesFootball scoreboard test-mode live rendering
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Harness as Test harness
participant Manager as manager.py
participant SportsLive as SportsLive.update
participant TestMode as _test_mode_update
Harness->>Manager: Provide NFL test_mode=true
Manager->>SportsLive: Create scoreboard configuration
SportsLive->>TestMode: Advance simulated game
SportsLive-->>Harness: Render updated live game without network fetch
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 29 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
plugins/football-scoreboard/test_live_screen.py (1)
46-55: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExercise the runtime contracts.
test_config_passthrough(),test_update_short_circuits_in_test_mode(), andtest_seeded_live_game_is_complete()only inspect source text. They do not invoke_adapt_config_for_manager(),SportsLive.update(), orNFLLiveManager. A dead branch or an update that still calls_fetch_data()can pass these checks. Add runtime assertions with an overdue update, a failing fetch stub, and field-level checks on the seeded game.Also applies to: 57-68, 80-98
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugins/football-scoreboard/test_live_screen.py` around lines 46 - 55, Replace the source-text checks in test_config_passthrough(), test_update_short_circuits_in_test_mode(), and test_seeded_live_game_is_complete() with runtime tests that invoke _adapt_config_for_manager(), SportsLive.update(), and NFLLiveManager. Use an overdue update and a fetch stub that fails if called to verify test-mode short-circuiting, then assert the seeded game’s required fields and values directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/football-scoreboard/test_live_screen.py`:
- Line 33: Remove the unused deferred imports of manager, sports, and
nfl_managers and eliminate the PLUGIN_DIR sys.path mutation in the affected test
functions. If imports are required for coverage, load them through
plugin-unique, namespaced module paths and assert the resulting module objects;
do not bind bare shared names.
In `@plugins/football-scoreboard/test/harness.json`:
- Around line 2-10: Adjust the test fixture’s live-update setup so the first
render is overdue and executes SportsLive.update(), allowing _test_mode_update()
to run. Replace the effectively infinite live_update_interval with a small
finite interval, or initialize SportsLive.last_update accordingly before
rendering, while preserving the seeded offline live-game behavior.
---
Nitpick comments:
In `@plugins/football-scoreboard/test_live_screen.py`:
- Around line 46-55: Replace the source-text checks in
test_config_passthrough(), test_update_short_circuits_in_test_mode(), and
test_seeded_live_game_is_complete() with runtime tests that invoke
_adapt_config_for_manager(), SportsLive.update(), and NFLLiveManager. Use an
overdue update and a fetch stub that fails if called to verify test-mode
short-circuiting, then assert the seeded game’s required fields and values
directly.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a813d0b5-e502-4a5b-8d3d-ea60a32e4706
📒 Files selected for processing (6)
plugins.jsonplugins/football-scoreboard/manager.pyplugins/football-scoreboard/manifest.jsonplugins/football-scoreboard/sports.pyplugins/football-scoreboard/test/harness.jsonplugins/football-scoreboard/test_live_screen.py
Vestigial: the checks read the source as text rather than importing the modules, and importing them here would need the core on the path anyway. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
The fixture kept live_update_interval at 9999999999, inherited from when live mode was disabled and the always-instantiated live manager had to be stopped from attempting a doomed network fetch. With last_update at 0 and time frozen at 2026-01-15, the first update was therefore never overdue: the harness rendered the seeded game straight from NFLLiveManager.__init__ and never ran the short-circuit this change adds. test_mode returns before any fetch, so the guard is obsolete. The interval is a normal 30s and the fixture now exercises the path it exists to cover. Adds the behavioural test that was missing alongside the source checks: drive the real SportsLive.update() with test_mode on and assert _test_mode_update() ran, no fetch happened, and the seeded game survived. Deleting the short-circuit makes it fail. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
2 participants
The season starts in a couple of weeks and live is the screen that carries it — yet it was the one screen the safety harness never rendered. The fixture said why:
Both halves of that turned out to be bugs, not gaps.
1.
test_modewas unreachable_adapt_config_for_manager()builds each manager's config fromgame_limits,display_options,filteringanddisplay_modes— and never passedtest_modethrough. SoSportsLive.test_modecould not be set from config at all, which made unreachable:NFLLiveManager.__init__— score, Q4 clock, down and distance, possession, timeouts, all of it already writtenncaa_fb_managers.py:185'sif self.test_modebranch2.
_test_mode_update()was dead codeSportsLive.update()fetched unconditionally, so even with the flag set the seeded game was overwritten on the first tick.FootballLive._test_mode_update()existed and was never called.baseball-scoreboardhad already fixed exactly this — itstest_test_mode_live_games.pyasserts "test_mode=True must call_test_mode_update(), NOT_fetch_data()" — so the short-circuit is ported from there rather than invented.Result
The harness now renders
nfl_liveat every panel size: 16 renders → 24, none blank (verified against the empty-render check from #447). The card draws the parts only this mode has:No behaviour change for anyone running it.
test_modedefaults toFalseon both paths, so live fetches exactly as before — the short-circuit is only reachable when a fixture opts in.Why now
This is the same shape as two bugs already found this month: hockey's
display()returnedNoneon every path and hid until the off-season emptied its modes, and this plugin's ownhas_live_contentlog flood was — per its test docstring — "dormant only because it was the off-season", with baseball measuring 13,871 lines/minute with nine live games. Live-path bugs don't show up until there are live games. Two weeks out is the time to render that screen in CI rather than on opening Sunday.Verification
test_live_screen.py: 17 checks — the config passthrough exists,update()branches ontest_modebefore the fetch, the harness actually enables live, and the seeded game carries every field a live scorebug draws (a fixture missing the clock or down-and-distance would render something that passes while covering nothing).🤖 Generated with Claude Code
https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5
Summary by CodeRabbit