feat(scoreboards): add scroll mode config to baseball, football, basketball#38
Closed
ChuckBuilds wants to merge 9 commits intomainfrom
Closed
feat(scoreboards): add scroll mode config to baseball, football, basketball#38ChuckBuilds wants to merge 9 commits intomainfrom
ChuckBuilds wants to merge 9 commits intomainfrom
Conversation
Comprehensive F1 racing plugin with 8 display modes: - Driver standings, constructor standings - Recent race results with podium visualization - Upcoming race with countdown timer and circuit outline - Qualifying results (Q1/Q2/Q3 with times and gaps) - Free practice standings, sprint race results - Season calendar with session schedule Features: - Three API sources: ESPN (schedule), Jolpi (standings/results), OpenF1 (practice) - Favorite team and driver filtering - Team color accent bars and podium gold/silver/bronze - Bundled team logos (10) and circuit layout images (24) - Pre-season fallback to previous season data - Fully dynamic layout for any matrix size (64x32 to 192x48+) - Vegas scroll integration Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Inline fixes: - Fix cached dict mutation in apply_favorite_filter (shallow copy entries) - Return [] from except block in standings fetch to prevent caching errors - Remove hardcoded dev path from font loading - Add _truncate_text helper and enforce text_max_x in render_upcoming_race - Remove dead 'madring' key from CIRCUIT_FILENAME_MAP - Pass force_clear through in _display_upcoming - Bump package pins (requests>=2.32.0, Pillow>=10.4.0) Nitpick fixes: - Add additionalProperties: false to all 8 mode objects in config schema - Use explicit default_return param in _fallback_previous_season - Extract _get_latest_round helper, remove duplicated standings fetch - Fix Optional type hints in f1_renderer, scroll_display - Add debug logging for swallowed exceptions in logo_downloader - Remove blocking update()/prepare from get_vegas_content - Remove unused _prepare_time field and time import from scroll_display - Remove redundant self-referencing aliases from team_colors Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Inline fixes: - Compute qualifying elimination cutoffs from result count instead of hardcoding 15/10 thresholds for a 20-car grid - Shallow copy driver standings entries before adding poles key - Shallow copy recent race dicts before mutating results - Shallow copy practice/sprint results before slicing - Bump Pillow to >=12.1.1 for CVE-2026-25990 Nitpick fixes: - Guard getattr in _fallback_previous_season against misspelled methods - Memoize _get_latest_round to avoid redundant HTTP requests across calculate_pole_positions, fetch_recent_races, fetch_qualifying, etc. - Initialize time_width=0 before conditional block in qualifying render - Extract _render_driver_row helper from render_qualifying_entry, render_practice_entry, and render_sprint_entry (-130 lines) - Fix get_team_logo return type to Image.Image (always returns image) - Sort CIRCUIT_FILENAME_MAP keys longest-first in _resolve_circuit_filename - Add get_vegas_items() public accessor on ScrollDisplay - Log warning on ScrollHelper import failure Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Inline fix: - Remove dead ESPN_SESSION_TYPES and ESPN_SESSION_ABBRS constants Nitpick fixes: - Remove redundant .lower().replace() before normalize_constructor_id - Populate _latest_round_cache from fetch_driver_standings to avoid duplicate HTTP requests from _get_latest_round - Single bounded fallback in _fallback_previous_season (always targets current_year - 1 instead of recursing through each year) - Use _truncate_text in calendar entry rendering for consistent ellipsis - Fix _load_logo return type to Image.Image (always returns via fallback) - Fix preload_all_teams log to count files iterated, not total cache size - Isolate each update step in own try/except so one failure doesn't abort remaining data fetches - Static fallback in display_scroll_frame: show first card when ScrollHelper is unavailable Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Bring the F1 plugin up to parity with mature sports plugins: - display() returns bool so empty modes are skipped in rotation - Dynamic duration support (scroll modes run until cycle completes) - on_config_change rebuilds ScrollDisplayManager - get_info() provides diagnostic data for web UI - Vegas content filtered to only modes with data - Config schema: x-propertyOrder, x-widget checkbox-group, enum dropdowns for favorites, title on all fields, dynamic_duration block Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Guard display_manager.matrix access with hasattr check - Shallow-copy upcoming race data to avoid mutating cache - Reuse parsed datetime instead of parsing twice - Config-driven ScrollHelper params with sensible defaults - Add ScrollDisplay.is_scroll_complete() delegation method - Add ScrollDisplayManager.get_vegas_items_for_mode() public accessor - Use self.logger.exception() in cleanup for traceback capture - Fix display_duration type from number to integer - Remove hardcoded enum arrays for favorite_team/driver (stale risk) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…etball Adds `live_display_mode`, `recent_display_mode`, and `upcoming_display_mode` fields (enum: switch|scroll, default: switch) to each league's display_modes block in config_schema.json, plus a `scroll_settings` block with scroll_speed, scroll_delay, gap_between_games, show_league_separators, and dynamic_duration. All three plugins already had full scroll mode code infrastructure in manager.py; this change exposes those settings in the web UI so users can enable scroll mode on large displays where switch mode leaves excessive blank space. Hockey and soccer scoreboards were already complete (unchanged). Bumped versions: baseball 1.3.1→1.3.2, football 2.1.1→2.1.2, basketball 1.1.1→1.1.2 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a new F1 Scoreboard plugin (data, rendering, logos, manager, scrolling, colors) and extends baseball, basketball, and football scoreboard plugins with per-league display modes ("switch"/"scroll"), scroll settings, customization options, and version bumps; updates the plugin registry entry and timestamps. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant F1ScoreboardPlugin
participant F1DataSource
participant ESPN_API
participant Jolpi_API
participant F1LogoLoader
participant F1Renderer
participant ScrollDisplayManager
participant DisplayManager
User->>F1ScoreboardPlugin: update()
F1ScoreboardPlugin->>F1DataSource: fetch standings/races/qualifying/calendar
F1DataSource->>Jolpi_API: get driver/constructor standings & results
F1DataSource->>ESPN_API: get schedule/upcoming race
Jolpi_API-->>F1DataSource: standings/results JSON
ESPN_API-->>F1DataSource: schedule JSON
F1ScoreboardPlugin->>F1LogoLoader: preload/get logos/circuits
F1ScoreboardPlugin->>F1Renderer: render cards (standings, results, upcoming)
F1Renderer->>F1LogoLoader: get_team_logo / get_circuit_image
F1LogoLoader-->>F1Renderer: image assets or placeholders
F1Renderer-->>F1ScoreboardPlugin: PIL Image cards
F1ScoreboardPlugin->>ScrollDisplayManager: prepare_and_display(cards)
ScrollDisplayManager->>DisplayManager: display_scroll_frame / render static card
DisplayManager-->>User: content shown on LED matrix
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
plugins/football-scoreboard/config_schema.json (2)
604-999:⚠️ Potential issue | 🔴 Critical
customizationis placed outsideproperties— it will be ignored by JSON Schema validators and rejected in config data.The
customizationobject (line 604) is a sibling of"properties"at the schema root, not nested inside it. In JSON Schema draft-07, unrecognized root-level keywords are silently ignored. Combined with"additionalProperties": falseon line 1000, any actualcustomizationkey in user config will be rejected by validation.Move
customizationinside the"properties"block (before line 603's closing brace).🐛 Proposed fix
} } - } - }, - "customization": { + }, + "customization": { "type": "object", ... + } }, "additionalProperties": false,i.e., close
ncaa_fband keepcustomizationas a property inside the rootpropertiesobject, then closeproperties.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/football-scoreboard/config_schema.json` around lines 604 - 999, The schema's "customization" object is defined at the root instead of being declared as a property, so validators will ignore it and configs will be rejected due to "additionalProperties": false; move the "customization" block into the root "properties" object (i.e., make "customization" a member of "properties" rather than a sibling) ensuring it is closed inside that object and that the surrounding braces for the existing root property (e.g., the "ncaa_fb"/root properties block) remain balanced; after moving, keep "additionalProperties": false intact and update the "x-propertyOrder" if needed so "customization" appears in the intended order.
1-5:⚠️ Potential issue | 🟡 MinorVersion bump should be MINOR (2.2.0), not PATCH (2.1.2).
The football-scoreboard plugin was bumped from 2.1.1 → 2.1.2, but the config_schema.json introduces new backward-compatible config options:
display_modesenums (switch/scrollmodes for live, recent, and upcoming games) andscroll_settings(scroll_speed, scroll_delay, gap_between_games, show_league_separators, dynamic_duration) for both NFL and NCAA Football sections. Per the coding guidelines, new config options require a MINOR version bump: 2.1.1 → 2.2.0.Update
manifest.jsonaccordingly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/football-scoreboard/config_schema.json` around lines 1 - 5, The manifest version bump is incorrect — adding new backward-compatible config options requires a MINOR bump. Update the plugin manifest version from 2.1.2 to 2.2.0 (and any other places using the old version) to reflect the new config_schema.json additions: the new enums/fields display_modes (for live/recent/upcoming) and the scroll_settings object (scroll_speed, scroll_delay, gap_between_games, show_league_separators, dynamic_duration) for NFL and NCAA sections; ensure manifest.json's "version" and any published version metadata are changed to 2.2.0 so the release correctly denotes a minor version bump.plugins/basketball-scoreboard/config_schema.json (1)
1-6:⚠️ Potential issue | 🟡 MinorVersion bump should be MINOR (1.2.0), not PATCH (1.1.2).
The manifest.json shows the basketball-scoreboard version is bumped to 1.1.2 (PATCH). However, this PR adds new backward-compatible config options including display mode enums (
live_display_mode,recent_display_mode,upcoming_display_modewith "switch"/"scroll" options) and scroll settings (scroll_speed,scroll_delay,gap_between_games,show_league_separators,dynamic_duration) across all four leagues. This qualifies as a MINOR version bump per the project's versioning guidelines.The version should be bumped to 1.2.0 instead of 1.1.2.
Per coding guidelines: "Bump MINOR version (1.x.0) for new features added, new config options (backward compatible), or new display modes/functionality."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/basketball-scoreboard/config_schema.json` around lines 1 - 6, Update the plugin version from 1.1.2 to 1.2.0 in the manifest/version field for the basketball-scoreboard plugin to reflect the addition of new backward-compatible config options (live_display_mode, recent_display_mode, upcoming_display_mode, scroll_speed, scroll_delay, gap_between_games, show_league_separators, dynamic_duration); modify the "version" value in the manifest.json (the basketball-scoreboard manifest entry) and any other metadata files or packaging manifests that reference 1.1.2 so all references consistently use 1.2.0.
🧹 Nitpick comments (5)
plugins/basketball-scoreboard/config_schema.json (1)
150-154:dynamic_durationnaming collision may confuse users in the web UI.Each league section now has two properties named
dynamic_duration: a boolean here inscroll_settings(auto-calculate scroll display duration based on content width) and a separate object at the league level (e.g., Line 277 for NBA) that controls per-game dynamic duration calculation. Both will render with the same label in the web UI.Consider renaming this boolean to something more specific like
auto_scroll_durationto disambiguate it from the existing per-leaguedynamic_durationobject.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/basketball-scoreboard/config_schema.json` around lines 150 - 154, The boolean property "dynamic_duration" under the scroll_settings object conflicts with the existing per-league "dynamic_duration" object; rename the scroll_settings key to "auto_scroll_duration", update its default and description to "Automatically calculate scroll display duration based on content width" (or similar), and update any code/config references that access scroll_settings.dynamic_duration to use scroll_settings.auto_scroll_duration so the web UI label is unambiguous relative to the per-league dynamic_duration object.plugins/baseball-scoreboard/config_schema.json (1)
124-128: Potential naming confusion:scroll_settings.dynamic_duration(boolean) vs league-leveldynamic_duration(object).Within
scroll_settings,dynamic_durationis a boolean (line 124–128). At the league level (e.g., MLB line 264),dynamic_durationis a complex object withenabled,min_duration_seconds,max_duration_seconds, and per-modemodes. While these don't collide in the schema (different nesting), they may confuse users in the web UI since they share the same property name with different semantics. Consider renaming one — e.g.,auto_scroll_durationinsidescroll_settings.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/baseball-scoreboard/config_schema.json` around lines 124 - 128, The boolean field scroll_settings.dynamic_duration conflicts semantically with the league-level dynamic_duration object; rename the boolean to a clearer name (e.g., scroll_settings.auto_scroll_duration) and update all references to that symbol in the codebase (config parsing, validation, UI bindings) so that scroll_settings.auto_scroll_duration is used instead of scroll_settings.dynamic_duration while leaving the league-level dynamic_duration object intact; ensure schema description and any docs/UI labels are updated to match the new name.plugins/f1-scoreboard/manifest.json (1)
1-34: Consider addingrepo,branch, andplugin_pathfields for consistency.Other plugin manifests (e.g., baseball-scoreboard, basketball-scoreboard) include
repo,branch, andplugin_pathin their manifests. This manifest omits them. Not strictly required, but would improve consistency across the monorepo.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/f1-scoreboard/manifest.json` around lines 1 - 34, Update the plugin manifest JSON (manifest.json for id "f1-scoreboard", class_name "F1ScoreboardPlugin") to include the missing repository metadata fields used by other plugins: add top-level string fields "repo" (git URL or repo slug), "branch" (default branch name like "main"), and "plugin_path" (relative path to the plugin within the repo). Place these fields alongside the existing top-level keys (e.g., near "id"/"name") and ensure values follow the same format used by other manifests in the repo for consistency.plugins/f1-scoreboard/manager.py (1)
531-545:_MODE_KEY_MAPcould be atypes.MappingProxyTypefor immutability, but this is fine as-is.Ruff flags this (RUF012) because it's a mutable dict on a class attribute, but it's used strictly as a read-only lookup table alongside the already-frozen
_SCROLL_MODES. Low risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/f1-scoreboard/manager.py` around lines 531 - 545, The class-level lookup _MODE_KEY_MAP is mutable and triggers a linter (RUF012); make it immutable by importing types and wrapping the dict literal with types.MappingProxyType to create a read-only mapping (i.e., replace the _MODE_KEY_MAP assignment with a MappingProxyType(...) containing the same key/value pairs) while keeping _SCROLL_MODES unchanged; ensure you add the import for types at the top of the module and update any references to _MODE_KEY_MAP (they should continue to work since MappingProxyType preserves dict-like access).plugins/f1-scoreboard/f1_data.py (1)
775-806:calculate_pole_positionsmay be slow at end-of-season with up to ~24 sequential HTTP requests.On first invocation each cache cycle, this fetches qualifying for every completed round (
range(1, current_round + 1)). Late in the season, that's ~24 sequential HTTP calls. While individually cached, a cold start (e.g., after restart) will block_update_standingsfor a long time.Consider fetching qualifying results in a batch or lazily populating poles as qualifying data is fetched by other modes (e.g., during
_update_qualifying).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/f1-scoreboard/f1_data.py` around lines 775 - 806, calculate_pole_positions currently does up to current_round sequential fetch_qualifying calls on a cold cache, causing long blocking during _update_standings; change it to avoid serial HTTP calls by either (A) building the pole counts from any already-cached qualifying payloads (use _get_cached for per-round qualifying entries) before issuing network requests, or (B) fetch remaining missing rounds concurrently (e.g., threadpool/async gather) instead of a simple for-loop; update the function to consult _get_cached for each round, only call fetch_qualifying for rounds not cached, accumulate poles by driver code as now, and persist with _set_cached; alternatively consider moving pole aggregation into _update_qualifying so qualifying updates incrementally update the "f1_poles_{season}" cache rather than recalculating all rounds in calculate_pole_positions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins.json`:
- Around line 224-227: Update the synchronized latest_version fields in
plugins.json for the affected monorepo plugins: set the football plugin's
"latest_version" to "2.2.0", the basketball plugin's "latest_version" to
"1.2.0", and the baseball plugin's "latest_version" to "1.4.0"; ensure you
update every occurrence of the "latest_version" key for these plugins (including
the other entries noted in the review) so they match the corrected manifest
versions and keep the registry in sync.
In `@plugins/baseball-scoreboard/manifest.json`:
- Line 4: Update the manifest version string to perform a MINOR bump because new
backward-compatible config fields were added; locate the "version" entry in
manifest.json (the "version": "1.3.2" key/value) and change it to "1.4.0" (and
similarly update the other occurrences noted around lines 33-37) so the package
correctly reflects a minor feature release.
In `@plugins/basketball-scoreboard/manifest.json`:
- Line 4: Update the manifest "version" field from "1.1.2" to "1.2.0" in the
plugins/basketball-scoreboard manifest and change any other occurrences of the
same "version": "1.1.2" entry in that manifest (the other entries around the
same block) to "1.2.0" so the release reflects a MINOR bump for added
config/display options.
In `@plugins/f1-scoreboard/manager.py`:
- Around line 456-471: Remove the unused local variable next_session_dt: delete
its initial declaration and the assignment inside the for loop so the code only
computes and sets upcoming["countdown_seconds"] and
upcoming["next_session_type"] when a future session is found; specifically
remove the line initializing next_session_dt = None and the line next_session_dt
= parsed_dt inside the loop (keep the datetime parsing, comparison, setting of
upcoming fields, and the break).
In `@plugins/f1-scoreboard/manifest.json`:
- Line 28: The manifest.json for the f1-scoreboard plugin has a "last_updated"
value ("2026-02-17") that is inconsistent with the entry in plugins.json
("2026-02-18"); update the "last_updated" field so both places match (pick the
correct canonical date used in plugins.json or update plugins.json to match if
that is intended) by editing the "last_updated" key in
plugins/f1-scoreboard/manifest.json (and verify the plugins.json entry with the
same plugin identifier) to ensure both show the identical date string.
In `@plugins/football-scoreboard/manifest.json`:
- Line 4: Update the manifest.json version from "2.1.2" to "2.2.0" and also
update the matching entry inside the "versions" array (the entries referenced
around lines 27-31) so they reflect 2.2.0; this is a MINOR bump to indicate new
backward-compatible config options (live_display_mode, recent_display_mode,
upcoming_display_mode enums and the scroll_settings block).
---
Outside diff comments:
In `@plugins/basketball-scoreboard/config_schema.json`:
- Around line 1-6: Update the plugin version from 1.1.2 to 1.2.0 in the
manifest/version field for the basketball-scoreboard plugin to reflect the
addition of new backward-compatible config options (live_display_mode,
recent_display_mode, upcoming_display_mode, scroll_speed, scroll_delay,
gap_between_games, show_league_separators, dynamic_duration); modify the
"version" value in the manifest.json (the basketball-scoreboard manifest entry)
and any other metadata files or packaging manifests that reference 1.1.2 so all
references consistently use 1.2.0.
In `@plugins/football-scoreboard/config_schema.json`:
- Around line 604-999: The schema's "customization" object is defined at the
root instead of being declared as a property, so validators will ignore it and
configs will be rejected due to "additionalProperties": false; move the
"customization" block into the root "properties" object (i.e., make
"customization" a member of "properties" rather than a sibling) ensuring it is
closed inside that object and that the surrounding braces for the existing root
property (e.g., the "ncaa_fb"/root properties block) remain balanced; after
moving, keep "additionalProperties": false intact and update the
"x-propertyOrder" if needed so "customization" appears in the intended order.
- Around line 1-5: The manifest version bump is incorrect — adding new
backward-compatible config options requires a MINOR bump. Update the plugin
manifest version from 2.1.2 to 2.2.0 (and any other places using the old
version) to reflect the new config_schema.json additions: the new enums/fields
display_modes (for live/recent/upcoming) and the scroll_settings object
(scroll_speed, scroll_delay, gap_between_games, show_league_separators,
dynamic_duration) for NFL and NCAA sections; ensure manifest.json's "version"
and any published version metadata are changed to 2.2.0 so the release correctly
denotes a minor version bump.
---
Nitpick comments:
In `@plugins/baseball-scoreboard/config_schema.json`:
- Around line 124-128: The boolean field scroll_settings.dynamic_duration
conflicts semantically with the league-level dynamic_duration object; rename the
boolean to a clearer name (e.g., scroll_settings.auto_scroll_duration) and
update all references to that symbol in the codebase (config parsing,
validation, UI bindings) so that scroll_settings.auto_scroll_duration is used
instead of scroll_settings.dynamic_duration while leaving the league-level
dynamic_duration object intact; ensure schema description and any docs/UI labels
are updated to match the new name.
In `@plugins/basketball-scoreboard/config_schema.json`:
- Around line 150-154: The boolean property "dynamic_duration" under the
scroll_settings object conflicts with the existing per-league "dynamic_duration"
object; rename the scroll_settings key to "auto_scroll_duration", update its
default and description to "Automatically calculate scroll display duration
based on content width" (or similar), and update any code/config references that
access scroll_settings.dynamic_duration to use
scroll_settings.auto_scroll_duration so the web UI label is unambiguous relative
to the per-league dynamic_duration object.
In `@plugins/f1-scoreboard/f1_data.py`:
- Around line 775-806: calculate_pole_positions currently does up to
current_round sequential fetch_qualifying calls on a cold cache, causing long
blocking during _update_standings; change it to avoid serial HTTP calls by
either (A) building the pole counts from any already-cached qualifying payloads
(use _get_cached for per-round qualifying entries) before issuing network
requests, or (B) fetch remaining missing rounds concurrently (e.g.,
threadpool/async gather) instead of a simple for-loop; update the function to
consult _get_cached for each round, only call fetch_qualifying for rounds not
cached, accumulate poles by driver code as now, and persist with _set_cached;
alternatively consider moving pole aggregation into _update_qualifying so
qualifying updates incrementally update the "f1_poles_{season}" cache rather
than recalculating all rounds in calculate_pole_positions.
In `@plugins/f1-scoreboard/manager.py`:
- Around line 531-545: The class-level lookup _MODE_KEY_MAP is mutable and
triggers a linter (RUF012); make it immutable by importing types and wrapping
the dict literal with types.MappingProxyType to create a read-only mapping
(i.e., replace the _MODE_KEY_MAP assignment with a MappingProxyType(...)
containing the same key/value pairs) while keeping _SCROLL_MODES unchanged;
ensure you add the import for types at the top of the module and update any
references to _MODE_KEY_MAP (they should continue to work since MappingProxyType
preserves dict-like access).
In `@plugins/f1-scoreboard/manifest.json`:
- Around line 1-34: Update the plugin manifest JSON (manifest.json for id
"f1-scoreboard", class_name "F1ScoreboardPlugin") to include the missing
repository metadata fields used by other plugins: add top-level string fields
"repo" (git URL or repo slug), "branch" (default branch name like "main"), and
"plugin_path" (relative path to the plugin within the repo). Place these fields
alongside the existing top-level keys (e.g., near "id"/"name") and ensure values
follow the same format used by other manifests in the repo for consistency.
…, f1 cleanup - Bump baseball/football/basketball to minor versions (1.4.0, 2.2.0, 1.2.0) since adding new config fields is a backward-compatible feature addition - Fix football config_schema.json: move 'customization' block from top-level into 'properties' so it is recognized under additionalProperties:false - Remove unused next_session_dt variable in f1-scoreboard manager.py - Fix f1-scoreboard manifest: correct last_updated date (2026-02-17 -> 2026-02-18) and add missing repo/branch/plugin_path fields for registry consistency Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Owner
Author
|
Closing in favor of ##39 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
plugins/f1-scoreboard/manager.py (1)
535-543: Silence Ruff RUF012: annotate_MODE_KEY_MAPwithClassVar.
_MODE_KEY_MAPis a plaindictclass attribute flagged by Ruff RUF012. Mutable class-level defaults share state across all instances, and mutations in one instance unexpectedly affect all others. Even though this mapping is treated as a read-only lookup table, the annotation is needed to satisfy the linter (and to make intent explicit).ClassVarneeds to be added to thetypingimport on line 12.♻️ Proposed fix
-from typing import Any, Dict, List, Optional +from typing import Any, ClassVar, Dict, List, Optional- _MODE_KEY_MAP = { + _MODE_KEY_MAP: ClassVar[Dict[str, str]] = { "f1_driver_standings": "driver_standings", "f1_constructor_standings": "constructor_standings", "f1_recent_races": "recent_races", "f1_qualifying": "qualifying", "f1_practice": "practice", "f1_sprint": "sprint", "f1_calendar": "calendar", }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/f1-scoreboard/manager.py` around lines 535 - 543, Annotate the class-level mapping _MODE_KEY_MAP as a ClassVar to silence Ruff RUF012 and make the intent explicit: add ClassVar to the typing imports and change the type of _MODE_KEY_MAP to ClassVar[dict[str, str]] (or the appropriate mapping type) so the class attribute is treated as non-instance state; leave the runtime dict literal unchanged. Ensure the file's import line that currently imports from typing includes ClassVar.plugins/football-scoreboard/config_schema.json (1)
122-126: Consider renamingscroll_settings.dynamic_durationto avoid naming ambiguity.Within the same
nfl(andncaa_fb) block,scroll_settings.dynamic_durationis a boolean ("auto-size scroll duration from content width"), whilenfl.dynamic_durationat sibling level is an object (per-mode switch-timing settings). Both share the leaf namedynamic_duration, which can cause confusion when reading or hand-editing a config file.♻️ Suggested rename for both leagues
- "dynamic_duration": { - "type": "boolean", - "default": true, - "description": "Automatically calculate display duration based on content width" - } + "auto_scroll_duration": { + "type": "boolean", + "default": true, + "description": "Automatically calculate display duration based on content width" + }Apply the same rename to
ncaa_fb.scroll_settings.Also applies to: 407-411
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/football-scoreboard/config_schema.json` around lines 122 - 126, Rename the boolean field scroll_settings.dynamic_duration to a clearer name (e.g., scroll_settings.auto_duration or scroll_settings.dynamic_scroll_duration) to avoid confusion with the sibling object nfl.dynamic_duration; update the same rename under ncaa_fb.scroll_settings as well; adjust any references in the schema and consumer code that read/write scroll_settings.dynamic_duration so they use the new field name, and leave nfl.dynamic_duration (the per-mode object) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/f1-scoreboard/manager.py`:
- Around line 512-515: get_vegas_content is passing the raw self._upcoming_race
directly to renderer.render_upcoming_race, bypassing the dynamic countdown logic
in _display_upcoming so Vegas shows stale/empty countdowns; extract the
countdown computation from _display_upcoming into a small helper (e.g.,
_enrich_upcoming_with_countdown) that takes the upcoming race dict,
shallow-copies and sets countdown_seconds based on the nearest future session,
and call that helper from both _display_upcoming and get_vegas_content before
calling renderer.render_upcoming_race to ensure the rendered upcoming card
always has a fresh countdown.
---
Duplicate comments:
In `@plugins/f1-scoreboard/manager.py`:
- Around line 439-474: The previous unused variable next_session_dt reported by
Ruff (F841) has already been removed from _display_upcoming, so no code change
is needed; simply acknowledge the resolved issue and keep the current logic that
computes parsed_dt and sets upcoming["countdown_seconds"] and
upcoming["next_session_type"] in the _display_upcoming method.
---
Nitpick comments:
In `@plugins/f1-scoreboard/manager.py`:
- Around line 535-543: Annotate the class-level mapping _MODE_KEY_MAP as a
ClassVar to silence Ruff RUF012 and make the intent explicit: add ClassVar to
the typing imports and change the type of _MODE_KEY_MAP to ClassVar[dict[str,
str]] (or the appropriate mapping type) so the class attribute is treated as
non-instance state; leave the runtime dict literal unchanged. Ensure the file's
import line that currently imports from typing includes ClassVar.
In `@plugins/football-scoreboard/config_schema.json`:
- Around line 122-126: Rename the boolean field scroll_settings.dynamic_duration
to a clearer name (e.g., scroll_settings.auto_duration or
scroll_settings.dynamic_scroll_duration) to avoid confusion with the sibling
object nfl.dynamic_duration; update the same rename under
ncaa_fb.scroll_settings as well; adjust any references in the schema and
consumer code that read/write scroll_settings.dynamic_duration so they use the
new field name, and leave nfl.dynamic_duration (the per-mode object) unchanged.
Comment on lines
+512
to
+515
| if self._upcoming_race: | ||
| upcoming_card = self.renderer.render_upcoming_race( | ||
| self._upcoming_race) | ||
| images.insert(0, upcoming_card) |
There was a problem hiding this comment.
get_vegas_content renders the upcoming card without a fresh countdown.
_display_upcoming computes countdown_seconds dynamically on each call (shallow-copies self._upcoming_race, initialises countdown_seconds = None, then iterates sessions to find the nearest future one). get_vegas_content bypasses that logic and passes the raw self._upcoming_race dict directly to the renderer, so Vegas mode will always show a stale or absent countdown.
🐛 Proposed fix
- if self._upcoming_race:
- upcoming_card = self.renderer.render_upcoming_race(
- self._upcoming_race)
- images.insert(0, upcoming_card)
+ if self._upcoming_race:
+ upcoming = dict(self._upcoming_race)
+ upcoming["countdown_seconds"] = None
+ now = datetime.now(timezone.utc)
+ for session in upcoming.get("sessions", []):
+ if session.get("status_state") == "pre" and session.get("date"):
+ try:
+ parsed_dt = datetime.fromisoformat(
+ session["date"].replace("Z", "+00:00"))
+ if parsed_dt > now:
+ upcoming["countdown_seconds"] = max(
+ 0, (parsed_dt - now).total_seconds())
+ upcoming["next_session_type"] = session.get(
+ "type_abbr", "")
+ break
+ except (ValueError, TypeError):
+ continue
+ upcoming_card = self.renderer.render_upcoming_race(upcoming)
+ images.insert(0, upcoming_card)Alternatively, extract the countdown-computation logic from _display_upcoming into a small helper (e.g., _enrich_upcoming_with_countdown) and call it from both places.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/f1-scoreboard/manager.py` around lines 512 - 515, get_vegas_content
is passing the raw self._upcoming_race directly to
renderer.render_upcoming_race, bypassing the dynamic countdown logic in
_display_upcoming so Vegas shows stale/empty countdowns; extract the countdown
computation from _display_upcoming into a small helper (e.g.,
_enrich_upcoming_with_countdown) that takes the upcoming race dict,
shallow-copies and sets countdown_seconds based on the nearest future session,
and call that helper from both _display_upcoming and get_vegas_content before
calling renderer.render_upcoming_race to ensure the rendered upcoming card
always has a fresh countdown.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
live_display_mode,recent_display_mode, andupcoming_display_mode(enum:switch|scroll, default:switch) to each league'sdisplay_modesblock inconfig_schema.jsonfor baseball, football, and basketball scoreboardsscroll_settingsblock per league withscroll_speed,scroll_delay,gap_between_games,show_league_separators, anddynamic_durationmanager.py— this change exposes those settings in the web UITest plan
live_display_mode/recent_display_mode/upcoming_display_modedropdowns with "switch" / "scroll" options in MLB settingsjournalctl -u ledmatrixfor no errors🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
New Options
Enhancements
Updates