Merged
Conversation
Complete rewrite of the Masters Tournament plugin with: - 23 real ESPN player headshots (Tiger, Scheffler, McIlroy, Rahm, etc.) - 18 accurate Augusta National hole layouts with real course topology (doglegs, water hazards, Rae's Creek, Hogan Bridge, bunkers) - 16 country flags for player cards - Authentic Masters branding (logo with US map, green jacket, azaleas) - Pixel-perfect BDF/TTF font system using LEDMatrix font library - 14 display modes (was 10): leaderboard, player cards, hole cards, Amen Corner, past champions, fun facts, records, countdown, field overview, course overview, tee times, live alerts - Paginated displays with page indicator dots - Generous spacing for LED readability (5 players/page vs 7) - 35 real Masters fun facts, 40 past champions through 2025 - Complete tournament records database - Scrolling text for long fun facts - Player card rotation through top 5 - Front Nine / Back Nine course overview pages - Scales across 32x16, 64x32, and 128x64 displays Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace generated Masters logo with official logo from masters.com - Replace generated hole layouts with real overhead satellite/aerial maps from masters.com for all 18 holes - White backgrounds removed with alpha transparency - Scaled logo variants (sm/md/lg) for different display sizes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new "Masters Tournament" LEDMatrix plugin including asset generation and loader, ESPN-backed data source with mock mode, helpers/datasets, two renderers (base + enhanced), a plugin manager with mode/rotation/Vegas output, manifest/schema, README, and requirements. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Manager as MastersTournamentPlugin
participant Data as MastersDataSource
participant Assets as MastersLogoLoader
participant Renderer as MastersRenderer/Enhanced
participant Display as LEDMatrix
Client->>Manager: request update/display
Manager->>Data: fetch_leaderboard()/fetch_schedule()
Data-->>Manager: leaderboard/schedule (cached or live/mock)
Manager->>Assets: get_masters_logo/get_player_headshot/get_hole_image
Assets-->>Manager: images (cached or generated)
Manager->>Renderer: render_{mode}(data, images)
Renderer-->>Manager: PIL.Image frame(s)
Manager->>Display: push frame(s) / get_vegas_content
Display-->>Client: visual output shown
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
- Increase headshot size from 24px to 28px - Replace thumbnail() with crop-to-fill: crops ESPN headshots to square from top-center (where faces are) then resizes to fill the box - No more empty space inside the gold border Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🤖 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/masters-tournament/__init__.py`:
- Line 8: Update the module-level __version__ constant in
plugins/masters-tournament/__init__.py to match the version in the manifest
(change __version__ from "1.0.0" to "2.0.0") so logs and release docs are
consistent with plugins/masters-tournament/manifest.json.
In `@plugins/masters-tournament/config_schema.json`:
- Around line 39-257: The display_modes schema in config_schema.json is missing
four modes (fun_facts, countdown, field_overview, course_overview) declared in
manifest.json and adds per-mode duration keys that manager.py currently ignores;
update config_schema.json to include the four missing mode objects and remove or
align per-mode duration fields with the plugin runtime, and update manager.py's
_build_enabled_modes() (and any logic that reads display_duration) to recognize
"course_overview" and to either read each mode's duration_* setting or continue
using the top-level display_duration consistently; specifically modify the
display_modes object (and its child entries like leaderboard, player_cards,
course_tour, etc.) to match manifest.json's full list of mode names and then
update manager.py::_build_enabled_modes and any code that references
display_duration to consume the same per-mode key names you choose.
In `@plugins/masters-tournament/download_assets.py`:
- Around line 61-71: ESPN_PLAYERS contains a duplicate ID (both "Jordan Spieth"
and "Tommy Fleetwood" => "5765") which causes download_player_headshots() to
overwrite cached files named "<pid>.png"; fix by ensuring each ESPN_PLAYERS
value is unique (correct Tommy Fleetwood’s ID or remove the duplicate) or change
download_player_headshots() to write files using a unique key (e.g., include a
sanitized player name like "<pid>_<sanitized_name>.png" or use a mapping from
name→filename) so cached headshots cannot collide; update references to
ESPN_PLAYERS and the filename-generation logic in download_player_headshots()
accordingly.
In `@plugins/masters-tournament/logo_loader.py`:
- Around line 34-45: The code currently creates and writes asset directories
under self.plugin_dir (self.logos_dir, self.courses_dir, self.players_dir,
self.flags_dir, self.icons_dir, self.backgrounds_dir) which assumes the plugin
install path is writable; change the storage location to a proper writable cache
or data directory (e.g., use an application cache path, platform-specific user
cache dir, or tempfile.gettempdir()) and update all places that create
directories and save downloads (the directory.mkdir block and the download/save
logic referenced around the later download code) to use the new cache path
instead of plugin_dir/assets/...; ensure you still create the subfolders (logos,
courses, players, flags, icons, backgrounds) under that writable cache and make
directory.mkdir(parents=True, exist_ok=True) calls there.
- Around line 168-181: The logo download on cache miss is blocking the render
path (requests.get in the logo-loading block), so change the logic in the logo
loader to be non-blocking: have the load path (the method using
cache_key/self._cache and called from render_player_card()/get_vegas_content())
return a placeholder immediately when url is present but not cached, and
schedule an asynchronous/background fetch (thread, executor, or asyncio task)
that performs requests.get(...), Image.open(...).convert("RGBA"),
img.save(player_path, "PNG"), img.thumbnail(..., Image.Resampling.LANCZOS) and
then updates self._cache[cache_key]; ensure concurrent fetches for the same
cache_key are coalesced (e.g., track in-flight futures) and failures are logged
but not raised to the render path.
In `@plugins/masters-tournament/manager.py`:
- Around line 94-96: The featured-holes mode is reading the shared cursor
_current_hole and never advances its own pointer, causing stale repeated frames;
add a dedicated per-mode index (e.g., self._featured_hole_index) initialized
alongside self._current_hole in the constructor and update
masters_featured_holes to read/advance that index (with wrap-around to the
course length) instead of touching or relying on _current_hole so this mode no
longer couples to other modes; also replace any other uses that expect a
separate featured cursor (the same pattern referenced near the other block) to
use the new _featured_hole_index.
- Around line 288-289: The masters_hole_by_hole handler is incorrectly
delegating to _display_leaderboard(), so hole-by-hole mode never renders hole
cards or advances holes; modify _display_hole_by_hole to implement the
hole-by-hole flow instead of returning _display_leaderboard(force_clear): it
should render the current hole (use whatever rendering helper exists or add one,
e.g., _render_hole or _render_hole_cards), advance the hole index/state for the
next invocation, handle force_clear for screen refresh, and only fall back to
_display_leaderboard() when the hole-by-hole sequence is complete; update any
state variables the handler relies on (current_hole, hole_sequence_active, etc.)
and ensure masters_hole_by_hole triggers this updated _display_hole_by_hole.
- Around line 304-305: _replace the direct passthrough in _display_live_action
so it checks display capabilities (same guard pattern used for course overview)
and routes to the enhanced live-alert renderer when supported, otherwise fall
back to _display_leaderboard; update the _display_live_action implementation to
perform the capability check, call the enhanced live-alert renderer (the new
live-action screen) when the device supports 64x32+ or the required feature, and
only call _display_leaderboard as the fallback so masters_live_action can reach
the new renderer on capable displays.
- Around line 324-330: The countdown target is hard-coded to datetime(2026,...)
and naive (no timezone); change _display_countdown to compute the next
tournament datetime dynamically (use current UTC now to pick April 10 at 12:00
in the current year or the next year if that date/time has passed) and construct
a timezone-aware UTC datetime before calling calculate_tournament_countdown.
Update the call site in _display_countdown (which uses renderer.render_countdown
and _show_image) to pass the computed aware target instead of the fixed 2026
value.
In `@plugins/masters-tournament/manifest.json`:
- Around line 1-38: Add a top-level "versions" array to manifest.json and insert
a new first entry that documents the current release: include "released" (ISO
date), "version" equal to the existing top-level "version" ("2.0.0"), and
"ledmatrix_min" (minimum supported LED matrix firmware/format). Ensure the
top-level "version" value stays identical to the first element's "version" in
"versions", and keep the array ordered newest-to-oldest so future releases are
prepended.
In `@plugins/masters-tournament/masters_data.py`:
- Around line 246-254: The _detect_tournament_phase function currently
hard-codes April 7–13 which is incorrect for other years; replace this logic to
derive phase from the official event schedule instead: fetch or accept the
tournament schedule object (e.g., use the existing schedule/source that contains
event dates or a TournamentSchedule/masters_schedule variable) and compute
whether now falls in practice, tournament, or off-season using those actual
start/end dates, then return the appropriate string; also update any cache TTL
logic that relies on _detect_tournament_phase to use the schedule-derived dates
so cache expiry tracks the real event timeline rather than fixed April dates.
- Around line 219-223: The code currently assumes events[0] is the Masters;
change it to search the events list for the Masters event before reading
competitions/teeTimes. In masters_data.py, replace the events[0] usage with a
selection step that finds the correct event (e.g., iterate events and choose the
one where event.get("name", "").lower() or event.get("slug", "").lower()
contains "masters" or matches a known tournament id), then use that
selected_event.get("competitions", []) and iterate its comps to get teeTimes; if
no matching event is found, skip processing (or return empty) to avoid showing
non-Masters tee times.
- Around line 56-59: When _is_masters_tournament(data) returns False, the code
returns _generate_mock_leaderboard() without caching; modify that branch to
store the generated mock response in the same cache used for real tournament
responses (using the same cache key and a 3600-second TTL) before returning it
so off-season results are cached for one hour; update the branch that currently
logs "Masters not currently in ESPN API, using mock data" to call the caching
method (the one used elsewhere in this module for live responses) with the value
from _generate_mock_leaderboard() and TTL=3600, then return the cached/mock
payload.
In `@plugins/masters-tournament/masters_helpers.py`:
- Around line 286-297: get_tournament_phase currently hard-codes April 7–13;
replace that logic with a year-aware lookup so phases reflect actual tournament
dates: create a MASTERS_SCHEDULE mapping (e.g., dict keyed by year ->
{practice_start, tournament_start, tournament_end}) or implement a
get_masters_dates(year) helper, then update get_tournament_phase(date:
Optional[datetime]) to derive year = date.year, fetch that year's dates (with
sensible fallback), and return "practice"/"tournament"/"off-season" by comparing
date to the schedule ranges; reference get_tournament_phase and the new
MASTERS_SCHEDULE/get_masters_dates symbols when locating code to change.
- Around line 189-220: The ESPN_PLAYER_IDS map has duplicate id "5765" used for
"Jordan Spieth", "Tommy Fleetwood", and "Danny Willett", causing
headshot/caching collisions; open the ESPN_PLAYER_IDS dict and replace the
incorrect duplicate entries by looking up and setting the correct unique ESPN id
values for "Tommy Fleetwood" and "Danny Willett" (leave "Jordan Spieth" as-is if
his id is correct), then scan ESPN_PLAYER_IDS to ensure every player has a
unique "id" string and update any dependent headshot/cache keys that rely on
these ids.
In `@plugins/masters-tournament/masters_renderer.py`:
- Around line 148-189: The configured hardcoded max_players in _configure_tier
overflows the vertical space for each tier; replace the magic constants with a
computed value that fits the vertical budget: compute available_height =
<tier_height> - self.header_height - self.footer_height, then set
self.max_players = max(1, floor((available_height + self.row_gap) /
(self.row_height + self.row_gap))). Update the tiny/small/large branches in
_configure_tier to compute max_players this way (use 16, 32, 64 as the tier
heights shown in comments) and also fix the same logic referenced around the
other block (lines mentioned in the comment) so any code that uses
header_height/footer_height + 2 for initial y aligns with the new computed
max_players.
- Around line 660-703: In render_fun_fact, visible_lines can be 0 on tiny
screens so clamp it to at least 1 before using it in the start_line calculation
and slice; e.g., after computing visible_lines = (self.height - content_top - 4)
// line_h, set visible_lines = max(1, visible_lines) and then compute start_line
using max(1, len(lines) - visible_lines + 1) (or otherwise avoid
modulo/divide-by-zero) so the slice lines[start_line : start_line +
visible_lines] always returns at least one line; alternatively detect very small
panels in render_fun_fact and render a single-line ticker instead.
- Around line 516-520: The call to get_hole_image can pass a negative max_height
when rendering tiny-tier displays (e.g., 32x16) which leads
_create_hole_placeholder() to call Image.new with a negative height; clamp the
dimensions to a minimum of 1 before calling get_hole_image (e.g., compute
safe_max_height = max(1, self.height - h - 14) and safe_max_width = max(1,
self.width - 8) and pass those), or alternatively add the same min-1 guard
inside LogoLoader.get_hole_image/_create_hole_placeholder; update the invocation
around hole_img = self.logo_loader.get_hole_image(...) and/or the
_create_hole_placeholder implementation to ensure no negative/zero dimensions
are passed.
In `@plugins/masters-tournament/README.md`:
- Around line 7-18: The README still lists "10 Display Modes" and an initial
release of "1.0.0" but the plugin manifest was updated to 2.0.0 with 14 modes;
update the README.md to match the manifest by changing the version from 1.0.0 to
2.0.0 (or remove the hardcoded version) and add the four missing display
modes—fun_facts, countdown, field_overview, and course_overview—so the list
reflects all 14 modes and the heading/count is consistent with the manifest.
In `@plugins/masters-tournament/requirements.txt`:
- Line 2: The requirements currently allow Pillow>=10.0.0 which includes
vulnerable 10.0.0 releases; update the dependency line in requirements.txt that
references "Pillow>=10.0.0" to "Pillow>=10.2.0" so the plugin uses the earliest
patched release (ensure no other place pins an older Pillow version and run
dependency install/tests to verify compatibility).
🪄 Autofix (Beta)
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
Run ID: d2976fca-e032-412e-b6fc-d224b1602aee
⛔ Files ignored due to path filters (70)
plugins/masters-tournament/assets/masters/backgrounds/augusta_green_texture.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/backgrounds/masters_green_gradient.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/backgrounds/masters_green_gradient_128x64.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/backgrounds/masters_green_gradient_64x32.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/courses/hole_01.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/courses/hole_02.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/courses/hole_03.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/courses/hole_04.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/courses/hole_05.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/courses/hole_06.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/courses/hole_07.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/courses/hole_08.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/courses/hole_09.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/courses/hole_10.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/courses/hole_11.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/courses/hole_12.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/courses/hole_13.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/courses/hole_14.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/courses/hole_15.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/courses/hole_16.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/courses/hole_17.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/courses/hole_18.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/flags/ARG.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/flags/AUS.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/flags/CAN.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/flags/ENG.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/flags/ESP.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/flags/FIJ.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/flags/GER.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/flags/IRL.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/flags/JPN.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/flags/NIR.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/flags/NOR.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/flags/RSA.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/flags/SCO.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/flags/SWE.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/flags/USA.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/flags/WAL.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/icons/golf_ball.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/icons/golf_flag.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/icons/golf_tee.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/logos/azalea.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/logos/green_jacket.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/logos/masters_logo.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/logos/masters_logo_full.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/logos/masters_logo_lg.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/logos/masters_logo_md.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/logos/masters_logo_sm.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/10134.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/10138.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/10140.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/10591.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/10592.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/308.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/3448.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/3470.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/367.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/3702.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/462.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/4686082.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/4686084.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/5548.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/5765.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/5860.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/6798.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/780.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/9037.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/9131.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/9478.pngis excluded by!**/*.pngplugins/masters-tournament/assets/masters/players/9780.pngis excluded by!**/*.png
📒 Files selected for processing (12)
plugins/masters-tournament/README.mdplugins/masters-tournament/__init__.pyplugins/masters-tournament/config_schema.jsonplugins/masters-tournament/download_assets.pyplugins/masters-tournament/logo_loader.pyplugins/masters-tournament/manager.pyplugins/masters-tournament/manifest.jsonplugins/masters-tournament/masters_data.pyplugins/masters-tournament/masters_helpers.pyplugins/masters-tournament/masters_renderer.pyplugins/masters-tournament/masters_renderer_enhanced.pyplugins/masters-tournament/requirements.txt
Comment on lines
+39
to
+257
| "display_modes": { | ||
| "type": "object", | ||
| "title": "Display Modes Configuration", | ||
| "description": "Control which display modes are enabled and their settings", | ||
| "properties": { | ||
| "leaderboard": { | ||
| "type": "object", | ||
| "title": "Leaderboard Display", | ||
| "properties": { | ||
| "enabled": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Show live leaderboard" | ||
| }, | ||
| "top_n": { | ||
| "type": "integer", | ||
| "default": 10, | ||
| "minimum": 1, | ||
| "maximum": 50, | ||
| "description": "Number of players to show on leaderboard" | ||
| }, | ||
| "show_favorites_always": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Always include favorite players even if outside top N" | ||
| }, | ||
| "duration": { | ||
| "type": "number", | ||
| "default": 25, | ||
| "minimum": 5, | ||
| "maximum": 120, | ||
| "description": "Display duration for leaderboard (seconds)" | ||
| } | ||
| } | ||
| }, | ||
| "player_cards": { | ||
| "type": "object", | ||
| "title": "Player Card Display", | ||
| "properties": { | ||
| "enabled": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Show individual player spotlight cards" | ||
| }, | ||
| "show_headshots": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Display player headshot photos" | ||
| }, | ||
| "duration_per_player": { | ||
| "type": "number", | ||
| "default": 15, | ||
| "minimum": 5, | ||
| "maximum": 60, | ||
| "description": "Time to show each player card (seconds)" | ||
| } | ||
| } | ||
| }, | ||
| "course_tour": { | ||
| "type": "object", | ||
| "title": "Course Tour Display", | ||
| "properties": { | ||
| "enabled": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Show rotating hole maps with course imagery" | ||
| }, | ||
| "show_animations": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Enable transitions and animations" | ||
| }, | ||
| "duration_per_hole": { | ||
| "type": "number", | ||
| "default": 15, | ||
| "minimum": 5, | ||
| "maximum": 60, | ||
| "description": "Time to show each hole (seconds)" | ||
| }, | ||
| "featured_holes": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "integer", | ||
| "minimum": 1, | ||
| "maximum": 18 | ||
| }, | ||
| "default": [12, 13, 16], | ||
| "description": "Featured holes to highlight (Amen Corner, par 3s)" | ||
| } | ||
| } | ||
| }, | ||
| "hole_by_hole": { | ||
| "type": "object", | ||
| "title": "Hole-by-Hole Scoring", | ||
| "properties": { | ||
| "enabled": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Show hole-by-hole scores for favorite players" | ||
| }, | ||
| "duration": { | ||
| "type": "number", | ||
| "default": 20, | ||
| "minimum": 5, | ||
| "maximum": 120, | ||
| "description": "Display duration (seconds)" | ||
| } | ||
| } | ||
| }, | ||
| "live_action": { | ||
| "type": "object", | ||
| "title": "Live Action Notifications", | ||
| "properties": { | ||
| "enabled": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Show real-time birdie/eagle notifications" | ||
| }, | ||
| "duration": { | ||
| "type": "number", | ||
| "default": 10, | ||
| "minimum": 3, | ||
| "maximum": 30, | ||
| "description": "Notification display duration (seconds)" | ||
| } | ||
| } | ||
| }, | ||
| "amen_corner": { | ||
| "type": "object", | ||
| "title": "Amen Corner Display", | ||
| "properties": { | ||
| "enabled": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Dedicated display for holes 11-13 (Amen Corner)" | ||
| }, | ||
| "duration": { | ||
| "type": "number", | ||
| "default": 20, | ||
| "minimum": 5, | ||
| "maximum": 120, | ||
| "description": "Display duration (seconds)" | ||
| } | ||
| } | ||
| }, | ||
| "featured_holes": { | ||
| "type": "object", | ||
| "title": "Featured Holes Display", | ||
| "properties": { | ||
| "enabled": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Show scoring on signature holes (12, 16)" | ||
| }, | ||
| "duration": { | ||
| "type": "number", | ||
| "default": 15, | ||
| "minimum": 5, | ||
| "maximum": 120, | ||
| "description": "Display duration (seconds)" | ||
| } | ||
| } | ||
| }, | ||
| "schedule": { | ||
| "type": "object", | ||
| "title": "Schedule Display", | ||
| "properties": { | ||
| "enabled": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Show tee times and pairings" | ||
| }, | ||
| "duration": { | ||
| "type": "number", | ||
| "default": 20, | ||
| "minimum": 5, | ||
| "maximum": 120, | ||
| "description": "Display duration (seconds)" | ||
| } | ||
| } | ||
| }, | ||
| "past_champions": { | ||
| "type": "object", | ||
| "title": "Past Champions Display", | ||
| "properties": { | ||
| "enabled": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Show historical Masters winners" | ||
| }, | ||
| "duration": { | ||
| "type": "number", | ||
| "default": 20, | ||
| "minimum": 5, | ||
| "maximum": 120, | ||
| "description": "Display duration (seconds)" | ||
| } | ||
| } | ||
| }, | ||
| "tournament_stats": { | ||
| "type": "object", | ||
| "title": "Tournament Statistics", | ||
| "properties": { | ||
| "enabled": { | ||
| "type": "boolean", | ||
| "default": true, | ||
| "description": "Show tournament records and statistics" | ||
| }, | ||
| "duration": { | ||
| "type": "number", | ||
| "default": 15, | ||
| "minimum": 5, | ||
| "maximum": 120, | ||
| "description": "Display duration (seconds)" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| }, |
There was a problem hiding this comment.
The display_modes schema is out of sync with the plugin surface.
manifest.json advertises 14 modes, but this object only exposes 10, so fun_facts, countdown, field_overview, and course_overview won't show up in schema-driven config UIs. In the other direction, the current manager.py only reads each mode's enabled flag plus the top-level display_duration, so the per-mode duration* knobs added here are ignored today; course_overview also isn't in _build_enabled_modes(), so that advertised mode can't enter rotation at all.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/masters-tournament/config_schema.json` around lines 39 - 257, The
display_modes schema in config_schema.json is missing four modes (fun_facts,
countdown, field_overview, course_overview) declared in manifest.json and adds
per-mode duration keys that manager.py currently ignores; update
config_schema.json to include the four missing mode objects and remove or align
per-mode duration fields with the plugin runtime, and update manager.py's
_build_enabled_modes() (and any logic that reads display_duration) to recognize
"course_overview" and to either read each mode's duration_* setting or continue
using the top-level display_duration consistently; specifically modify the
display_modes object (and its child entries like leaderboard, player_cards,
course_tour, etc.) to match manifest.json's full list of mode names and then
update manager.py::_build_enabled_modes and any code that references
display_duration to consume the same per-mode key names you choose.
Comment on lines
+34
to
+45
| self.plugin_dir = Path(plugin_dir) | ||
| self.masters_dir = self.plugin_dir / "assets" / "masters" | ||
| self.logos_dir = self.masters_dir / "logos" | ||
| self.courses_dir = self.masters_dir / "courses" | ||
| self.players_dir = self.masters_dir / "players" | ||
| self.flags_dir = self.masters_dir / "flags" | ||
| self.icons_dir = self.masters_dir / "icons" | ||
| self.backgrounds_dir = self.masters_dir / "backgrounds" | ||
|
|
||
| for directory in [self.logos_dir, self.courses_dir, self.players_dir, | ||
| self.flags_dir, self.icons_dir, self.backgrounds_dir]: | ||
| directory.mkdir(parents=True, exist_ok=True) |
There was a problem hiding this comment.
Don't persist downloads under the plugin install path.
Creating directories and saving headshots under plugin_dir/assets/... assumes the plugin tree is writable. In packaged or read-only deployments that either fails during initialization or quietly disables disk caching when the first download lands.
Also applies to: 157-177
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/masters-tournament/logo_loader.py` around lines 34 - 45, The code
currently creates and writes asset directories under self.plugin_dir
(self.logos_dir, self.courses_dir, self.players_dir, self.flags_dir,
self.icons_dir, self.backgrounds_dir) which assumes the plugin install path is
writable; change the storage location to a proper writable cache or data
directory (e.g., use an application cache path, platform-specific user cache
dir, or tempfile.gettempdir()) and update all places that create directories and
save downloads (the directory.mkdir block and the download/save logic referenced
around the later download code) to use the new cache path instead of
plugin_dir/assets/...; ensure you still create the subfolders (logos, courses,
players, flags, icons, backgrounds) under that writable cache and make
directory.mkdir(parents=True, exist_ok=True) calls there.
Comment on lines
+168
to
+181
| # Download from URL | ||
| if url: | ||
| try: | ||
| response = requests.get(url, timeout=5, headers={ | ||
| "User-Agent": "LEDMatrix Masters Plugin/2.0" | ||
| }) | ||
| response.raise_for_status() | ||
|
|
||
| img = Image.open(BytesIO(response.content)).convert("RGBA") | ||
| img.save(player_path, "PNG") | ||
|
|
||
| img.thumbnail((max_size, max_size), Image.Resampling.LANCZOS) | ||
| self._cache[cache_key] = img | ||
| return img |
There was a problem hiding this comment.
Keep network fetches off the render path.
On a cache miss this does a blocking HTTP request, and render_player_card() / get_vegas_content() can hit it while generating frames. A slow CDN response can stall the display loop for seconds per player.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/masters-tournament/logo_loader.py` around lines 168 - 181, The logo
download on cache miss is blocking the render path (requests.get in the
logo-loading block), so change the logic in the logo loader to be non-blocking:
have the load path (the method using cache_key/self._cache and called from
render_player_card()/get_vegas_content()) return a placeholder immediately when
url is present but not cached, and schedule an asynchronous/background fetch
(thread, executor, or asyncio task) that performs requests.get(...),
Image.open(...).convert("RGBA"), img.save(player_path, "PNG"),
img.thumbnail(..., Image.Resampling.LANCZOS) and then updates
self._cache[cache_key]; ensure concurrent fetches for the same cache_key are
coalesced (e.g., track in-flight futures) and failures are logged but not raised
to the render path.
Comment on lines
+148
to
+189
| def _configure_tier(self): | ||
| """Configure display parameters by size tier with generous spacing.""" | ||
| if self.tier == "tiny": # 32x16 | ||
| self.max_players = 2 | ||
| self.name_len = 8 | ||
| self.row_height = 7 | ||
| self.header_height = 7 | ||
| self.logo_size = 0 | ||
| self.show_pos_badge = False | ||
| self.show_thru = False | ||
| self.show_country = False | ||
| self.show_headshot = False | ||
| self.headshot_size = 0 | ||
| self.row_gap = 0 | ||
| self.footer_height = 0 | ||
| elif self.tier == "small": # 64x32 | ||
| self.max_players = 3 # Was 4 - breathe | ||
| self.name_len = 10 | ||
| self.row_height = 7 | ||
| self.header_height = 8 | ||
| self.logo_size = 10 | ||
| self.show_pos_badge = True | ||
| self.show_thru = True | ||
| self.show_country = False | ||
| self.show_headshot = False | ||
| self.headshot_size = 0 | ||
| self.row_gap = 1 # 1px gap between rows | ||
| self.footer_height = 5 # Page dots | ||
| else: # 128x64 | ||
| self.max_players = 5 # Was 7 - much more readable | ||
| self.name_len = 14 | ||
| self.row_height = 9 # Was 7 - more vertical space | ||
| self.header_height = 11 | ||
| self.logo_size = 18 | ||
| self.show_pos_badge = True | ||
| self.show_thru = True | ||
| self.show_country = True | ||
| self.show_headshot = True | ||
| self.headshot_size = 24 # Was 20 | ||
| self.row_gap = 1 # 1px gap between rows | ||
| self.footer_height = 6 # Page dots | ||
|
|
There was a problem hiding this comment.
Leaderboard row counts exceed the vertical budget.
With the current header/footer heights and y = self.header_height + 2, the configured max_players overflows on every tier: 32x16 only fits 1 row, 64x32 fits 2, and 128x64 fits 4 before the footer/page dots start colliding.
♻️ Immediate fix
if self.tier == "tiny": # 32x16
- self.max_players = 2
+ self.max_players = 1
@@
elif self.tier == "small": # 64x32
- self.max_players = 3
+ self.max_players = 2
@@
else: # 128x64
- self.max_players = 5
+ self.max_players = 4Also applies to: 298-327
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/masters-tournament/masters_renderer.py` around lines 148 - 189, The
configured hardcoded max_players in _configure_tier overflows the vertical space
for each tier; replace the magic constants with a computed value that fits the
vertical budget: compute available_height = <tier_height> - self.header_height -
self.footer_height, then set self.max_players = max(1, floor((available_height +
self.row_gap) / (self.row_height + self.row_gap))). Update the tiny/small/large
branches in _configure_tier to compute max_players this way (use 16, 32, 64 as
the tier heights shown in comments) and also fix the same logic referenced
around the other block (lines mentioned in the comment) so any code that uses
header_height/footer_height + 2 for initial y aligns with the new computed
max_players.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (5)
plugins/masters-tournament/masters_renderer.py (3)
698-702:⚠️ Potential issue | 🟠 MajorClamp
visible_linesbefore applying the scroll math.On
32x16,visible_linesbecomes0, so the slice emptieslinesand the fun-fact panel renders blank. A minimum of1keeps the screen populated and the scrolling math valid.📏 Minimal fix
- visible_lines = (self.height - content_top - 4) // line_h + visible_lines = max(1, (self.height - content_top - 4) // line_h)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/masters-tournament/masters_renderer.py` around lines 698 - 702, visible_lines can be 0 on small displays (e.g. 32x16), causing the slice to empty the fun-fact panel; clamp visible_lines to at least 1 before using it. Update the calculation in the renderer (the block that computes visible_lines using self.height, content_top and line_h) to set visible_lines = max(1, (self.height - content_top - 4) // line_h) and then use that clamped value when computing start_line (with scroll_offset and len(lines)) and slicing lines.
148-189:⚠️ Potential issue | 🟠 Major
max_playersstill exceeds the vertical budget.With
y = self.header_height + 2, the current2 / 3 / 5settings only fit 1 row on32x16, 2 on64x32, and 4 on128x64before the footer/page dots collide. Derive the limit from the remaining body height instead of hardcoding it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/masters-tournament/masters_renderer.py` around lines 148 - 189, The hardcoded max_players in _configure_tier violates vertical spacing; instead compute it from the available body height: calculate available_body = total_height - (self.header_height + self.footer_height + 2) (use the known canvas heights per tier: tiny=16, small=32, large=64), then set self.max_players = floor((available_body + self.row_gap) / (self.row_height + self.row_gap)) so rows and gaps fit without colliding with the footer; update the branches in _configure_tier to remove the fixed 2/3/5 values and derive max_players after setting header_height, footer_height, row_height, and row_gap for each tier.
516-520:⚠️ Potential issue | 🔴 CriticalTiny hole cards can request a negative image height.
On
32x16,self.height - h - 14evaluates to-5. That gets passed intoget_hole_image()and can break a supported mode with invalid image dimensions. Clamp both values to at least1, or switch the tiny tier to a text-only hole card.🩹 Minimal guard
+ max_width = max(1, self.width - 8) + max_height = max(1, self.height - h - 14) hole_img = self.logo_loader.get_hole_image( hole_number, - max_width=self.width - 8, - max_height=self.height - h - 14, + max_width=max_width, + max_height=max_height, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/masters-tournament/masters_renderer.py` around lines 516 - 520, The call to get_hole_image passes possibly-negative dimensions (max_width=self.width - 8, max_height=self.height - h - 14) which on tiny displays can become <1; clamp both computed dimensions to a minimum of 1 (or detect the tiny tier and use a text-only hole card) before calling get_hole_image. Update the code around the hole_img = self.logo_loader.get_hole_image(...) call to compute safe_width = max(1, self.width - 8) and safe_height = max(1, self.height - h - 14) (or branch to a text-only rendering path when in the tiny tier) and pass those safe values into get_hole_image.plugins/masters-tournament/logo_loader.py (2)
34-45:⚠️ Potential issue | 🟠 MajorMove runtime downloads out of the plugin tree.
self.masters_diris still rooted underself.plugin_dir, so packaged or read-only installs can fail duringmkdir()or later atimg.save(...). Because the save is inside the success path, a permission error also drops an otherwise successful headshot instead of rendering it from memory. Use a writable cache/data directory and make persistence best-effort after the cropped image has been cached.🗂️ Minimal direction for a safer cache flow
+import tempfile @@ - self.masters_dir = self.plugin_dir / "assets" / "masters" + self.masters_dir = Path(tempfile.gettempdir()) / "ledmatrix" / "masters" @@ - img.save(player_path, "PNG") - - img = self._crop_to_fill(img, max_size) - self._cache[cache_key] = img - return img + rendered = self._crop_to_fill(img, max_size) + self._cache[cache_key] = rendered + try: + img.save(player_path, "PNG") + except OSError as e: + logger.warning(f"Failed to persist headshot {player_id}: {e}") + return renderedAlso applies to: 188-193
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/masters-tournament/logo_loader.py` around lines 34 - 45, self.masters_dir is still under self.plugin_dir which can be read-only; change the runtime cache to a user-writable location (e.g. use appdirs.user_cache_dir or tempfile.gettempdir()) instead of deriving paths from self.plugin_dir, update the attributes (self.masters_dir, self.logos_dir, self.courses_dir, self.players_dir, self.flags_dir, self.icons_dir, self.backgrounds_dir) to point at that cache, and ensure directory creation is best-effort (wrap mkdir in try/except and continue on failure). Also make image persistence best-effort: when saving cropped images (the code that calls img.save), first keep the image in-memory and only attempt to write to the writable cache if available, catching and logging exceptions from img.save so a permission error does not discard or prevent returning the in-memory image.
180-195:⚠️ Potential issue | 🟠 MajorKeep headshot fetches off the render path.
requests.get(..., timeout=5)runs synchronously here, andrender_player_card()calls this method while building frames. A slow or failing CDN response can stall the display for seconds per uncached player, then do it again next frame because failures are not memoized. Return a placeholder immediately and fetch/cache in the background with in-flight/failure tracking.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/masters-tournament/logo_loader.py` around lines 180 - 195, The synchronous requests.get call inside the headshot fetch must be moved off the render path: when render_player_card() calls the loader (the block using cache_key, _cache, player_path, and _crop_to_fill), immediately return a placeholder if the image is not cached and kick off a background fetch instead; implement an _inflight set and a _failed map (with timestamps or backoff) to avoid spawning duplicate fetches or re-trying failed downloads every frame, and update _cache and save player_path from the background thread on success, or record the failure and logger.debug the error on failure; protect access to _cache/_inflight/_failed with a lock to be thread-safe.
🤖 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/masters-tournament/masters_renderer.py`:
- Around line 834-851: The vertical offsets for until_text, count_text and
unit_text (currently using fixed -6 / +4 / +16 relative to mid_y) cause clipping
on small/tiny tiers; update the layout in the block that sets mid_y, until_text,
cw, and uw2 to compute y positions dynamically using measured text heights (use
_text_width and font metrics / text size from self.font_detail and
self.font_score) or add explicit compact presets for tiers "small"/"tiny";
ensure count_text (used with _text_shadow) and unit_text are positioned inside
self.height (e.g., stack spacing = logo_height + until_height + count_height +
unit_height + padding) and clamp or reduce spacing for tiny to avoid drawing
off-canvas.
- Around line 744-759: The pagination computes visible rows using line_h but
each record actually uses two lines (label + value), so change the visible
calculation to divide the available height by the full record block height
(record_height = 2 * line_h) and then recompute total_pages, page wrap, start,
and records based on that new visible; update the variables visible,
total_pages, start, and records in masters_renderer.py (the block using visible,
total_pages, page, start, records and line_h) so slicing and page counts match
the actual per-record vertical space.
---
Duplicate comments:
In `@plugins/masters-tournament/logo_loader.py`:
- Around line 34-45: self.masters_dir is still under self.plugin_dir which can
be read-only; change the runtime cache to a user-writable location (e.g. use
appdirs.user_cache_dir or tempfile.gettempdir()) instead of deriving paths from
self.plugin_dir, update the attributes (self.masters_dir, self.logos_dir,
self.courses_dir, self.players_dir, self.flags_dir, self.icons_dir,
self.backgrounds_dir) to point at that cache, and ensure directory creation is
best-effort (wrap mkdir in try/except and continue on failure). Also make image
persistence best-effort: when saving cropped images (the code that calls
img.save), first keep the image in-memory and only attempt to write to the
writable cache if available, catching and logging exceptions from img.save so a
permission error does not discard or prevent returning the in-memory image.
- Around line 180-195: The synchronous requests.get call inside the headshot
fetch must be moved off the render path: when render_player_card() calls the
loader (the block using cache_key, _cache, player_path, and _crop_to_fill),
immediately return a placeholder if the image is not cached and kick off a
background fetch instead; implement an _inflight set and a _failed map (with
timestamps or backoff) to avoid spawning duplicate fetches or re-trying failed
downloads every frame, and update _cache and save player_path from the
background thread on success, or record the failure and logger.debug the error
on failure; protect access to _cache/_inflight/_failed with a lock to be
thread-safe.
In `@plugins/masters-tournament/masters_renderer.py`:
- Around line 698-702: visible_lines can be 0 on small displays (e.g. 32x16),
causing the slice to empty the fun-fact panel; clamp visible_lines to at least 1
before using it. Update the calculation in the renderer (the block that computes
visible_lines using self.height, content_top and line_h) to set visible_lines =
max(1, (self.height - content_top - 4) // line_h) and then use that clamped
value when computing start_line (with scroll_offset and len(lines)) and slicing
lines.
- Around line 148-189: The hardcoded max_players in _configure_tier violates
vertical spacing; instead compute it from the available body height: calculate
available_body = total_height - (self.header_height + self.footer_height + 2)
(use the known canvas heights per tier: tiny=16, small=32, large=64), then set
self.max_players = floor((available_body + self.row_gap) / (self.row_height +
self.row_gap)) so rows and gaps fit without colliding with the footer; update
the branches in _configure_tier to remove the fixed 2/3/5 values and derive
max_players after setting header_height, footer_height, row_height, and row_gap
for each tier.
- Around line 516-520: The call to get_hole_image passes possibly-negative
dimensions (max_width=self.width - 8, max_height=self.height - h - 14) which on
tiny displays can become <1; clamp both computed dimensions to a minimum of 1
(or detect the tiny tier and use a text-only hole card) before calling
get_hole_image. Update the code around the hole_img =
self.logo_loader.get_hole_image(...) call to compute safe_width = max(1,
self.width - 8) and safe_height = max(1, self.height - h - 14) (or branch to a
text-only rendering path when in the tiny tier) and pass those safe values into
get_hole_image.
🪄 Autofix (Beta)
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
Run ID: 41943cab-5b51-4fca-b598-32eadaa4bd94
📒 Files selected for processing (2)
plugins/masters-tournament/logo_loader.pyplugins/masters-tournament/masters_renderer.py
Comment on lines
+744
to
+759
| visible = max(1, (self.height - content_top - self.footer_height - 2) // line_h) | ||
| total_pages = max(1, (len(all_records) + visible - 1) // visible) | ||
| page = page % total_pages | ||
|
|
||
| start = page * visible | ||
| records = all_records[start : start + visible] | ||
|
|
||
| y = content_top | ||
| for label, value in records: | ||
| # Label in yellow | ||
| draw.text((3, y), label, fill=COLORS["masters_yellow"], font=font) | ||
| y += line_h - 1 | ||
|
|
||
| # Value indented in white | ||
| draw.text((6, y), value, fill=COLORS["white"], font=font) | ||
| y += line_h + 1 |
There was a problem hiding this comment.
Page capacity is computed for one text row, but each record uses two.
visible divides the available height by line_h, then every record advances y by 2 * line_h. That overestimates page capacity and pushes the last label/value pairs off-canvas. Base pagination on the full record block height.
📐 Minimal fix
- visible = max(1, (self.height - content_top - self.footer_height - 2) // line_h)
+ record_h = line_h * 2
+ visible = max(1, (self.height - content_top - self.footer_height - 2) // record_h)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| visible = max(1, (self.height - content_top - self.footer_height - 2) // line_h) | |
| total_pages = max(1, (len(all_records) + visible - 1) // visible) | |
| page = page % total_pages | |
| start = page * visible | |
| records = all_records[start : start + visible] | |
| y = content_top | |
| for label, value in records: | |
| # Label in yellow | |
| draw.text((3, y), label, fill=COLORS["masters_yellow"], font=font) | |
| y += line_h - 1 | |
| # Value indented in white | |
| draw.text((6, y), value, fill=COLORS["white"], font=font) | |
| y += line_h + 1 | |
| record_h = line_h * 2 | |
| visible = max(1, (self.height - content_top - self.footer_height - 2) // record_h) | |
| total_pages = max(1, (len(all_records) + visible - 1) // visible) | |
| page = page % total_pages | |
| start = page * visible | |
| records = all_records[start : start + visible] | |
| y = content_top | |
| for label, value in records: | |
| # Label in yellow | |
| draw.text((3, y), label, fill=COLORS["masters_yellow"], font=font) | |
| y += line_h - 1 | |
| # Value indented in white | |
| draw.text((6, y), value, fill=COLORS["white"], font=font) | |
| y += line_h + 1 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/masters-tournament/masters_renderer.py` around lines 744 - 759, The
pagination computes visible rows using line_h but each record actually uses two
lines (label + value), so change the visible calculation to divide the available
height by the full record block height (record_height = 2 * line_h) and then
recompute total_pages, page wrap, start, and records based on that new visible;
update the variables visible, total_pages, start, and records in
masters_renderer.py (the block using visible, total_pages, page, start, records
and line_h) so slicing and page counts match the actual per-record vertical
space.
Comment on lines
+834
to
+851
| mid_y = self.height // 2 | ||
|
|
||
| # "UNTIL THE MASTERS" centered | ||
| until_text = "UNTIL THE MASTERS" if self.tier != "tiny" else "TO MASTERS" | ||
| uw = self._text_width(draw, until_text, self.font_detail) | ||
| draw.text(((self.width - uw) // 2, mid_y - 6), | ||
| until_text, fill=COLORS["white"], font=self.font_detail) | ||
|
|
||
| # Big countdown number | ||
| cw = self._text_width(draw, count_text, self.font_score) | ||
| self._text_shadow(draw, ((self.width - cw) // 2, mid_y + 4), | ||
| count_text, self.font_score, COLORS["masters_yellow"]) | ||
|
|
||
| # Unit below | ||
| if unit_text: | ||
| uw2 = self._text_width(draw, unit_text, self.font_detail) | ||
| draw.text(((self.width - uw2) // 2, mid_y + 16), | ||
| unit_text, fill=COLORS["light_gray"], font=self.font_detail) |
There was a problem hiding this comment.
Countdown vertical offsets do not fit the small/tiny tiers.
unit_text is drawn at mid_y + 16, which is y = 32 on a 64x32 panel and entirely off-canvas. On 32x16, the count itself starts near the bottom after the logo and also clips. This needs a tier-specific compact layout or positions derived from actual logo/text heights instead of fixed -6 / +4 / +16 offsets.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/masters-tournament/masters_renderer.py` around lines 834 - 851, The
vertical offsets for until_text, count_text and unit_text (currently using fixed
-6 / +4 / +16 relative to mid_y) cause clipping on small/tiny tiers; update the
layout in the block that sets mid_y, until_text, cw, and uw2 to compute y
positions dynamically using measured text heights (use _text_width and font
metrics / text size from self.font_detail and self.font_score) or add explicit
compact presets for tiers "small"/"tiny"; ensure count_text (used with
_text_shadow) and unit_text are positioned inside self.height (e.g., stack
spacing = logo_height + until_height + count_height + unit_height + padding) and
clamp or reduce spacing for tiny to avoid drawing off-canvas.
Modes now change automatically based on tournament phase and time of day: - Off-season: fun facts, champions, course tour, countdown - Pre-tournament (late March/early April): countdown, fun facts, course tour, featured holes, champions - Practice rounds (Mon-Wed): schedule, course tour, fun facts, featured holes - Tournament morning (6-8am): schedule, leaderboard, field overview - Tournament LIVE (8am-7pm): leaderboard 3x per cycle, player cards, field overview, live action, featured holes, Amen Corner - Tournament evening (after 7pm): leaderboard, player cards, champions, records, fun facts - Post-tournament (Monday): leaderboard, player cards, champions, records Modes auto-refresh on each update cycle so transitions happen seamlessly as the day progresses. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
plugins/masters-tournament/manager.py (4)
96-98:⚠️ Potential issue | 🟠 MajorGive featured holes its own cursor.
Lines 397-398 read
_current_holeand never advance a featured-hole index, so this mode repeats the same hole untilcourse_tourmutates shared state. Because_current_holestarts at1, the first featured frame is also13instead of12.Minimal fix
# Course tour state self._current_hole = 1 + self._featured_hole_index = 0 @@ def _display_featured_holes(self, force_clear: bool) -> bool: featured = [12, 13, 15, 16] - hole = featured[self._current_hole % len(featured)] - return self._show_image(self.renderer.render_hole_card(hole)) + hole = featured[self._featured_hole_index % len(featured)] + result = self._show_image(self.renderer.render_hole_card(hole)) + self._featured_hole_index += 1 + return resultAlso applies to: 395-398
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/masters-tournament/manager.py` around lines 96 - 98, The featured-holes mode is reusing self._current_hole (which never advances for featured frames), causing repeats and an off-by-one start; add a separate cursor (e.g., self._current_featured_hole or self._featured_hole_index) initialized in __init__ alongside self._current_hole, update the code paths that select featured holes (the method that reads _current_hole for featured frames) to use this new cursor, and advance that cursor each time a featured frame is produced (wrap/limit it to the course length as needed) so featured holes progress independently of course_tour.
408-409:⚠️ Potential issue | 🟠 Major
masters_live_actionis still unreachable.Line 409 just returns leaderboard, so the live-action/live-alert screen added in this PR never renders even on 64x32+ displays. Please route this through the enhanced renderer with the same capability guard used by
_display_course_overview(), and keep leaderboard only as the fallback.Verify the available renderer entrypoints before wiring the guard:
#!/bin/bash fd -i 'masters_renderer.*\.py' plugins/masters-tournament | while IFS= read -r file; do echo "=== $file ===" rg -n 'render_.*(live|alert|action)|render_course_overview' "$file" done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/masters-tournament/manager.py` around lines 408 - 409, The method _display_live_action currently returns _display_leaderboard unconditionally, making masters_live_action unreachable; change it to use the enhanced renderer guard the same way _display_course_overview does (check the enhanced renderer capability/entrypoint for live/action/alert rendering and call that renderer, e.g., render_live_action or render_live_alert when available) and only fall back to calling _display_leaderboard if the enhanced renderer entrypoint is not present or the capability check fails; reference _display_live_action, _display_course_overview, _display_leaderboard and the renderer function names (e.g., masters_live_action / render_live_action) when locating where to add the capability check and conditional dispatch.
392-393:⚠️ Potential issue | 🟠 Major
masters_hole_by_holestill never renders a hole card.Line 393 proxies straight to
_display_leaderboard(), so this mode can never show the hole-by-hole sequence it advertises. It needs to render a hole card and advance hole state instead of delegating to leaderboard.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/masters-tournament/manager.py` around lines 392 - 393, The _display_hole_by_hole method currently just delegates to _display_leaderboard so the masters_hole_by_hole mode never shows hole cards; change _display_hole_by_hole to render the current hole card and advance hole state instead of calling _display_leaderboard: locate _display_hole_by_hole and replace the proxy call with logic that (1) calls the routine that draws a single hole card (e.g., existing render/emit method for a hole card or implement a short helper if needed), (2) advances the hole index/state (use the same state variables or helper used elsewhere to progress holes), and (3) returns the same boolean success/continue flag as _display_leaderboard did; keep _display_leaderboard untouched and only use it when you actually need the leaderboard view.
428-430:⚠️ Potential issue | 🟠 MajorMake the countdown target dynamic.
Line 429 hard-codes April 10, 2026. After April 10, 2026, this screen will return zeroes forever instead of counting down to the next Masters. Please compute the next tournament target from the current year/shared schedule helper and pass an aware timestamp into
calculate_tournament_countdown().Verify the fixed target and the helper contract together:
#!/bin/bash echo "Countdown call site:" sed -n '428,435p' plugins/masters-tournament/manager.py echo echo "Countdown helper contract:" sed -n '442,457p' plugins/masters-tournament/masters_helpers.py🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/masters-tournament/manager.py` around lines 428 - 430, Replace the hard-coded datetime in _display_countdown with a computed next-Masters target: call the shared schedule helper (e.g., a function like get_next_masters_date or next_masters_date in masters_helpers) to obtain the upcoming tournament date for the current year (or next year if the date has passed), convert that result into a timezone-aware datetime, and pass that aware timestamp into calculate_tournament_countdown(target); ensure you update _display_countdown to use the helper and produce an aware datetime rather than the fixed datetime so the countdown rolls over each year and matches the helper's contract.plugins/masters-tournament/masters_helpers.py (2)
286-297:⚠️ Potential issue | 🟠 MajorStop hard-coding one Masters week into the phase helpers.
Lines 291-295 and 324-350 still bake in a single April window. Masters week shifts year to year, so phase-driven rotation/screens will drift as soon as the actual schedule moves. Please drive both helpers from a shared year-aware schedule table instead of literal day ranges.
Also applies to: 321-350
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/masters-tournament/masters_helpers.py` around lines 286 - 297, The helper get_tournament_phase (and the other phase helper in the 321-350 range) currently hard-codes a single April window; replace those literal day checks with logic that looks up the year's Masters date range from a shared year-aware schedule (e.g., a MASTERS_SCHEDULE dict or a get_masters_schedule(year) function) and determine "practice", "tournament", or "off-season" by comparing the passed date to the returned start/practice/tournament date ranges; update both helpers to call the same schedule accessor so phase logic stays correct as the tournament dates shift year-to-year.
189-220:⚠️ Potential issue | 🟠 MajorFix the duplicate ESPN IDs before headshot caching.
"5765"is still assigned to Jordan Spieth, Tommy Fleetwood, and Danny Willett.get_espn_headshot_url()builds URLs directly from this map, so those players will collide on the same image/cache key and at least two headshots will be wrong.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/masters-tournament/masters_helpers.py` around lines 189 - 220, ESPN_PLAYER_IDS contains duplicate ESPN id "5765" used for Jordan Spieth, Tommy Fleetwood, and Danny Willett which causes get_espn_headshot_url() to generate colliding cache keys; update ESPN_PLAYER_IDS entries for Tommy Fleetwood and Danny Willett to their correct, unique ESPN ids (replace the duplicated "5765" with the proper ids) and verify all values in ESPN_PLAYER_IDS are unique before headshot caching (add a quick assertion or a small dedupe check near where get_espn_headshot_url() is used to fail fast if duplicates reappear).
🤖 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/masters-tournament/manager.py`:
- Around line 319-320: The fallback to the hardcoded "masters_leaderboard"
forces rendering when self.modes is empty; change the logic so that if
display_mode is None and self.modes is empty you leave display_mode as None
(i.e., do not default to "masters_leaderboard") and let the existing
_build_enabled_modes() / downstream checks handle the empty-mode case; update
the block that sets display_mode to use self.modes[0] if self.modes else None
and ensure callers handle None appropriately.
In `@plugins/masters-tournament/masters_helpers.py`:
- Around line 286-290: The functions get_tournament_phase and get_detailed_phase
must normalize input datetimes to the Augusta timezone before extracting
month/day/hour; update both to use ZoneInfo("America/New_York"): when date is
None call datetime.now(ZoneInfo("America/New_York")), otherwise call date =
date.astimezone(ZoneInfo("America/New_York")), then derive month/day/hour from
this normalized date so comparisons use the correct America/New_York calendar
boundaries.
---
Duplicate comments:
In `@plugins/masters-tournament/manager.py`:
- Around line 96-98: The featured-holes mode is reusing self._current_hole
(which never advances for featured frames), causing repeats and an off-by-one
start; add a separate cursor (e.g., self._current_featured_hole or
self._featured_hole_index) initialized in __init__ alongside self._current_hole,
update the code paths that select featured holes (the method that reads
_current_hole for featured frames) to use this new cursor, and advance that
cursor each time a featured frame is produced (wrap/limit it to the course
length as needed) so featured holes progress independently of course_tour.
- Around line 408-409: The method _display_live_action currently returns
_display_leaderboard unconditionally, making masters_live_action unreachable;
change it to use the enhanced renderer guard the same way
_display_course_overview does (check the enhanced renderer capability/entrypoint
for live/action/alert rendering and call that renderer, e.g., render_live_action
or render_live_alert when available) and only fall back to calling
_display_leaderboard if the enhanced renderer entrypoint is not present or the
capability check fails; reference _display_live_action,
_display_course_overview, _display_leaderboard and the renderer function names
(e.g., masters_live_action / render_live_action) when locating where to add the
capability check and conditional dispatch.
- Around line 392-393: The _display_hole_by_hole method currently just delegates
to _display_leaderboard so the masters_hole_by_hole mode never shows hole cards;
change _display_hole_by_hole to render the current hole card and advance hole
state instead of calling _display_leaderboard: locate _display_hole_by_hole and
replace the proxy call with logic that (1) calls the routine that draws a single
hole card (e.g., existing render/emit method for a hole card or implement a
short helper if needed), (2) advances the hole index/state (use the same state
variables or helper used elsewhere to progress holes), and (3) returns the same
boolean success/continue flag as _display_leaderboard did; keep
_display_leaderboard untouched and only use it when you actually need the
leaderboard view.
- Around line 428-430: Replace the hard-coded datetime in _display_countdown
with a computed next-Masters target: call the shared schedule helper (e.g., a
function like get_next_masters_date or next_masters_date in masters_helpers) to
obtain the upcoming tournament date for the current year (or next year if the
date has passed), convert that result into a timezone-aware datetime, and pass
that aware timestamp into calculate_tournament_countdown(target); ensure you
update _display_countdown to use the helper and produce an aware datetime rather
than the fixed datetime so the countdown rolls over each year and matches the
helper's contract.
In `@plugins/masters-tournament/masters_helpers.py`:
- Around line 286-297: The helper get_tournament_phase (and the other phase
helper in the 321-350 range) currently hard-codes a single April window; replace
those literal day checks with logic that looks up the year's Masters date range
from a shared year-aware schedule (e.g., a MASTERS_SCHEDULE dict or a
get_masters_schedule(year) function) and determine "practice", "tournament", or
"off-season" by comparing the passed date to the returned
start/practice/tournament date ranges; update both helpers to call the same
schedule accessor so phase logic stays correct as the tournament dates shift
year-to-year.
- Around line 189-220: ESPN_PLAYER_IDS contains duplicate ESPN id "5765" used
for Jordan Spieth, Tommy Fleetwood, and Danny Willett which causes
get_espn_headshot_url() to generate colliding cache keys; update ESPN_PLAYER_IDS
entries for Tommy Fleetwood and Danny Willett to their correct, unique ESPN ids
(replace the duplicated "5765" with the proper ids) and verify all values in
ESPN_PLAYER_IDS are unique before headshot caching (add a quick assertion or a
small dedupe check near where get_espn_headshot_url() is used to fail fast if
duplicates reappear).
🪄 Autofix (Beta)
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
Run ID: 509f66cc-c604-4ec2-8845-2ca7dcd19c39
📒 Files selected for processing (2)
plugins/masters-tournament/manager.pyplugins/masters-tournament/masters_helpers.py
Comment on lines
+286
to
+290
| def get_tournament_phase(date: Optional[datetime] = None) -> str: | ||
| """Determine current Masters tournament phase (basic).""" | ||
| if date is None: | ||
| date = datetime.now() | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "Phase helper implementations:"
sed -n '286,352p' plugins/masters-tournament/masters_helpers.py
echo
echo "Timezone normalization references:"
rg -n 'ZoneInfo|astimezone' plugins/masters-tournament/masters_helpers.py || trueRepository: ChuckBuilds/ledmatrix-plugins
Length of output: 2302
Normalize to Augusta (America/New_York) timezone before comparing calendar buckets.
Both get_tournament_phase() (lines 286-290) and get_detailed_phase() (lines 314-319) extract month, day, and hour from raw datetime.now() or caller-provided dates without timezone normalization. On hosts outside America/New_York, this causes the plugin to flip between phases at incorrect local times. Replace raw datetime comparisons with timezone-aware values converted to America/New_York:
from zoneinfo import ZoneInfo
if date is None:
date = datetime.now(ZoneInfo("America/New_York"))
else:
date = date.astimezone(ZoneInfo("America/New_York"))Then extract month, day, and hour from the normalized date.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/masters-tournament/masters_helpers.py` around lines 286 - 290, The
functions get_tournament_phase and get_detailed_phase must normalize input
datetimes to the Augusta timezone before extracting month/day/hour; update both
to use ZoneInfo("America/New_York"): when date is None call
datetime.now(ZoneInfo("America/New_York")), otherwise call date =
date.astimezone(ZoneInfo("America/New_York")), then derive month/day/hour from
this normalized date so comparisons use the correct America/New_York calendar
boundaries.
Verified each finding against current code. Applied fixes where needed: - __init__.py: bump __version__ to 2.0.0 to match manifest - config_schema.json: add 4 missing mode schemas (fun_facts, countdown, field_overview, course_overview) - download_assets.py: fix duplicate ESPN ID 5765 (Tommy Fleetwood now 9035, was sharing Spieth's ID) - masters_helpers.py: fix duplicate ESPN IDs for Tommy Fleetwood (9035) and Danny Willett (3008), no longer collide with Jordan Spieth (5765) - masters_renderer.py: clamp hole image dimensions to min 1px for tiny displays; clamp visible_lines to min 1 in fun facts to prevent divide-by-zero on small panels - manager.py: add dedicated _featured_hole_index so featured_holes mode doesn't share cursor with course_tour; implement _display_hole_by_hole to render actual holes instead of falling back to leaderboard; route _display_live_action to enhanced live-alert renderer when available; compute countdown target dynamically instead of hardcoded 2026 - masters_data.py: cache mock leaderboard (1hr TTL) when Masters isn't live to avoid regenerating on every call - requirements.txt: bump Pillow>=10.0.0 to >=10.2.0 (security patch) - README.md: update to 14 modes, add v2.0.0 changelog Skipped findings that were overengineered for the scope: - manifest.json versions array: not part of the plugin spec - logo_loader writable cache dir: plugin dir is writable by design - async headshot downloads: unnecessary complexity, downloads are cached - dynamic tournament schedule from API: hardcoded dates work fine, updated annually - computed max_players from vertical budget: current hardcoded values are tested and correct for each tier Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
plugins/masters-tournament/masters_renderer.py (2)
630-630: Unused loop variablecountry.The
countryvariable is unpacked but not used in the loop body. Rename to_countryto indicate intentional discard.- for i, (year, name, country, score) in enumerate(champs): + for i, (year, name, _country, score) in enumerate(champs):🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/masters-tournament/masters_renderer.py` at line 630, The loop unpacks (year, name, country, score) from champs but never uses country; update the for-loop in masters_renderer.py (the line "for i, (year, name, country, score) in enumerate(champs):") to use a throwaway name by renaming country to _country (i.e., "for i, (year, name, _country, score) in enumerate(champs):") so the unused variable intent is clear and lint warnings are resolved.
271-283: Silent exception swallowing in flag loading.The
except Exception: passat line 281-282 silently ignores all errors when loading flags. Consider at least debug-level logging for troubleshooting.except Exception: - pass + logger.debug(f"Failed to load flag {country_code}")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/masters-tournament/masters_renderer.py` around lines 271 - 283, The _get_flag method currently swallows all exceptions when loading flag images; modify the exception handler to log the error at debug level (include country_code and the exception message/trace) instead of silently passing so issues with Image.open/convert/thumbnail or path resolution can be diagnosed; locate _get_flag, the self._flag_cache lookup, and the code using self.flags_dir / f"{country_code}.png" and replace the bare "except Exception: pass" with a debug log call (e.g. using an existing logger on self or a module logger) and then return None as before.plugins/masters-tournament/download_assets.py (1)
395-395: Remove extraneousfprefix from string without placeholders.Several print statements use f-strings without any placeholders. While not a bug, it's cleaner to use regular strings.
♻️ Fix f-strings without placeholders
- print(f" [created] golf_ball.png") + print(" [created] golf_ball.png")Same applies to lines 409, 422, and 845.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/masters-tournament/download_assets.py` at line 395, Remove the unnecessary f-string prefixes on plain string print calls (e.g., replace print(f" [created] golf_ball.png") and the similar prints at the other noted locations) since there are no placeholders; update the print calls to use normal string literals (print(" [created] golf_ball.png")) where these statements appear so the output remains identical but the code is cleaner (check the corresponding print occurrences around the same block and at the other reported spots).
🤖 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/masters-tournament/masters_data.py`:
- Around line 100-109: fetch_player_details currently only checks cache
(cache_manager.get with cache_key built from player_id) and returns None on a
miss; implement a real network fetch or make the method explicitly cache-only.
To implement: after cache miss call the network client (e.g.,
self.api_client.get_player(player_id) or self.http_client.get(...) — whichever
exists in this class) to retrieve the player dict, validate the response, store
it back into cache with cache_manager.set(cache_key, data,
ttl=self._get_cache_ttl()), and return the data; if the project intends
cache-only instead, update the docstring of fetch_player_details to clearly
state it never fetches from network and adjust callers accordingly.
---
Nitpick comments:
In `@plugins/masters-tournament/download_assets.py`:
- Line 395: Remove the unnecessary f-string prefixes on plain string print calls
(e.g., replace print(f" [created] golf_ball.png") and the similar prints at the
other noted locations) since there are no placeholders; update the print calls
to use normal string literals (print(" [created] golf_ball.png")) where these
statements appear so the output remains identical but the code is cleaner (check
the corresponding print occurrences around the same block and at the other
reported spots).
In `@plugins/masters-tournament/masters_renderer.py`:
- Line 630: The loop unpacks (year, name, country, score) from champs but never
uses country; update the for-loop in masters_renderer.py (the line "for i,
(year, name, country, score) in enumerate(champs):") to use a throwaway name by
renaming country to _country (i.e., "for i, (year, name, _country, score) in
enumerate(champs):") so the unused variable intent is clear and lint warnings
are resolved.
- Around line 271-283: The _get_flag method currently swallows all exceptions
when loading flag images; modify the exception handler to log the error at debug
level (include country_code and the exception message/trace) instead of silently
passing so issues with Image.open/convert/thumbnail or path resolution can be
diagnosed; locate _get_flag, the self._flag_cache lookup, and the code using
self.flags_dir / f"{country_code}.png" and replace the bare "except Exception:
pass" with a debug log call (e.g. using an existing logger on self or a module
logger) and then return None as before.
🪄 Autofix (Beta)
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
Run ID: c3af57be-bf6a-482d-94d8-caa5fe6f3e79
📒 Files selected for processing (9)
plugins/masters-tournament/README.mdplugins/masters-tournament/__init__.pyplugins/masters-tournament/config_schema.jsonplugins/masters-tournament/download_assets.pyplugins/masters-tournament/manager.pyplugins/masters-tournament/masters_data.pyplugins/masters-tournament/masters_helpers.pyplugins/masters-tournament/masters_renderer.pyplugins/masters-tournament/requirements.txt
✅ Files skipped from review due to trivial changes (2)
- plugins/masters-tournament/init.py
- plugins/masters-tournament/config_schema.json
Comment on lines
+100
to
+109
| def fetch_player_details(self, player_id: str) -> Optional[Dict]: | ||
| """Fetch detailed player statistics.""" | ||
| cache_key = f"masters_player_{player_id}" | ||
| ttl = self._get_cache_ttl() | ||
|
|
||
| cached = self.cache_manager.get(cache_key, max_age=ttl) | ||
| if cached: | ||
| return cached | ||
|
|
||
| return None |
There was a problem hiding this comment.
fetch_player_details never fetches from network.
This method only returns cached data and returns None on cache miss. The manager calls this expecting player details but will always get None for uncached players. Either implement the network fetch or document this as cache-only.
📝 Suggested documentation or implementation
If intentionally cache-only:
def fetch_player_details(self, player_id: str) -> Optional[Dict]:
- """Fetch detailed player statistics."""
+ """Return cached player statistics (cache-only, no network fetch)."""
cache_key = f"masters_player_{player_id}"Or implement fetch:
def fetch_player_details(self, player_id: str) -> Optional[Dict]:
"""Fetch detailed player statistics."""
cache_key = f"masters_player_{player_id}"
ttl = self._get_cache_ttl()
cached = self.cache_manager.get(cache_key, max_age=ttl)
if cached:
return cached
- return None
+ # Player details are populated from leaderboard data
+ # No separate ESPN endpoint needed
+ return None🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/masters-tournament/masters_data.py` around lines 100 - 109,
fetch_player_details currently only checks cache (cache_manager.get with
cache_key built from player_id) and returns None on a miss; implement a real
network fetch or make the method explicitly cache-only. To implement: after
cache miss call the network client (e.g., self.api_client.get_player(player_id)
or self.http_client.get(...) — whichever exists in this class) to retrieve the
player dict, validate the response, store it back into cache with
cache_manager.set(cache_key, data, ttl=self._get_cache_ttl()), and return the
data; if the project intends cache-only instead, update the docstring of
fetch_player_details to clearly state it never fetches from network and adjust
callers accordingly.
- Phase helpers now normalize to Eastern Time (EDT UTC-4) before comparing hours, so a system in UTC/PST/etc still gets correct tournament-live vs tournament-evening boundaries - display() returns False when modes list is empty instead of forcing a hardcoded fallback to masters_leaderboard Skipped duplicate findings already fixed in previous commit: - _featured_hole_index: already separate (line 98) - _display_live_action: already routes to render_live_alert (line 411) - _display_hole_by_hole: already delegates to course_tour (line 393) - _display_countdown: already dynamic (line 442) - ESPN duplicate IDs: already fixed (Fleetwood=9035, Willett=3008) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
plugins/masters-tournament/masters_helpers.py (1)
301-311:⚠️ Potential issue | 🟠 MajorDrive phase detection from a year-aware Masters schedule.
These helpers still hard-code Apr 7-9 and Apr 10-13.
plugins/masters-tournament/manager.py:201-215usesget_detailed_phase()as the key forPHASE_MODES, so the plugin will enter practice/live/post-tournament on the wrong dates whenever Masters week shifts. Centralize the schedule in one lookup/helper and have both phase functions read from it.Also applies to: 314-365
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/masters-tournament/masters_helpers.py` around lines 301 - 311, get_tournament_phase currently hardcodes April dates; centralize Masters week into a single year-aware schedule lookup that both get_tournament_phase and get_detailed_phase consume: create or use a shared helper (e.g., get_masters_schedule(year) or MASTERS_SCHEDULE dict) that returns practice/tournament/post ranges for a given year, update get_tournament_phase to call that helper and determine phase from the returned ranges, and change get_detailed_phase (and any usage of PHASE_MODES) to read the same schedule so both functions stay in sync when Masters week shifts.plugins/masters-tournament/manager.py (1)
184-190:⚠️ Potential issue | 🟠 MajorCountdown shouldn't jump to next year while the tournament is still active.
masters_countdownis still part of thetournament-overnightrotation, but this target rolls forward as soon as a naivedatetime(year, 4, 10, 12, 0, 0)has passed. That means the screen can show next year's timer during the current Masters, and the naive timestamp is also interpreted as UTC bycalculate_tournament_countdown(). Reuse the same schedule source as the phase helpers and pass an aware datetime.Also applies to: 445-452
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/masters-tournament/manager.py` around lines 184 - 190, The countdown handler "masters_countdown" is being scheduled in the "tournament-overnight" rotation using a naive fixed datetime which rolls to the next year and is treated as UTC by calculate_tournament_countdown(); instead reuse the same schedule source used by the phase helpers (the tournament schedule/provider used by the phase helpers) to get the current tournament dates and pass an aware datetime (timezone-aware, e.g., local/ET) into calculate_tournament_countdown() so the countdown reflects the current tournament window and correct timezone; update the rotation entries to call the countdown with the schedule-derived aware datetime and adjust the code path that constructs the datetime to use the schedule/provider and timezone-aware conversion.
🤖 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/masters-tournament/manager.py`:
- Around line 125-198: PHASE_MODES is missing "masters_hole_by_hole", so the
masters_hole_by_hole screen (referenced in config_key_map and dispatch) never
appears in normal rotation; add "masters_hole_by_hole" to the appropriate phase
lists in PHASE_MODES (e.g., practice,
tournament-morning/tournament-live/tournament-evening as desired for UX) so it
can be selected during automatic cycling, ensuring the symbol name exactly
matches "masters_hole_by_hole" used in config_key_map/dispatch; update all three
PHASE_MODES blocks mentioned (lines ~125-198, ~212-227, ~331-346) consistently.
- Around line 279-293: The mock leaderboard entries use keys pos/name while
sort_leaderboard() and filter_favorite_players() expect position/player, causing
mock data to sort as 999 and favorites not to match; fix by normalizing entries
returned from self.data_source.fetch_leaderboard() (map 'pos'->'position' and
'name'->'player', and remove or coerce any invalid positions to an int) before
calling sort_leaderboard() and filter_favorite_players(), leaving the rest of
the logic (favorites, top_n, always_show) intact so both live and mock flows use
the same keys.
---
Duplicate comments:
In `@plugins/masters-tournament/manager.py`:
- Around line 184-190: The countdown handler "masters_countdown" is being
scheduled in the "tournament-overnight" rotation using a naive fixed datetime
which rolls to the next year and is treated as UTC by
calculate_tournament_countdown(); instead reuse the same schedule source used by
the phase helpers (the tournament schedule/provider used by the phase helpers)
to get the current tournament dates and pass an aware datetime (timezone-aware,
e.g., local/ET) into calculate_tournament_countdown() so the countdown reflects
the current tournament window and correct timezone; update the rotation entries
to call the countdown with the schedule-derived aware datetime and adjust the
code path that constructs the datetime to use the schedule/provider and
timezone-aware conversion.
In `@plugins/masters-tournament/masters_helpers.py`:
- Around line 301-311: get_tournament_phase currently hardcodes April dates;
centralize Masters week into a single year-aware schedule lookup that both
get_tournament_phase and get_detailed_phase consume: create or use a shared
helper (e.g., get_masters_schedule(year) or MASTERS_SCHEDULE dict) that returns
practice/tournament/post ranges for a given year, update get_tournament_phase to
call that helper and determine phase from the returned ranges, and change
get_detailed_phase (and any usage of PHASE_MODES) to read the same schedule so
both functions stay in sync when Masters week shifts.
🪄 Autofix (Beta)
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
Run ID: 31f2c10f-b659-4c8f-8bb3-549f89e23b69
📒 Files selected for processing (2)
plugins/masters-tournament/manager.pyplugins/masters-tournament/masters_helpers.py
Comment on lines
+279
to
+293
| raw_leaderboard = self.data_source.fetch_leaderboard() | ||
| if not raw_leaderboard: | ||
| return | ||
|
|
||
| sorted_board = sort_leaderboard(raw_leaderboard) | ||
|
|
||
| favorites = self.config.get("favorite_players", []) | ||
| top_n = self.config.get("display_modes", {}).get("leaderboard", {}).get("top_n", 10) | ||
| always_show = self.config.get("display_modes", {}).get("leaderboard", {}).get( | ||
| "show_favorites_always", True | ||
| ) | ||
|
|
||
| self._leaderboard_data = filter_favorite_players( | ||
| sorted_board, favorites, top_n=top_n, always_show_favorites=always_show | ||
| ) |
There was a problem hiding this comment.
Normalize mock leaderboard entries before calling the helpers.
sort_leaderboard() and filter_favorite_players() expect position and player, but plugins/masters-tournament/masters_data.py:122-178 and :270-310 emit position/player for live data and pos/name for mock data. In mock mode everyone sorts as 999, and favorite matching never fires.
♻️ Minimal fix in this method
- raw_leaderboard = self.data_source.fetch_leaderboard()
+ raw_leaderboard = [
+ {
+ **entry,
+ "position": entry.get("position", entry.get("pos")),
+ "player": entry.get("player", entry.get("name", "")),
+ }
+ for entry in self.data_source.fetch_leaderboard()
+ ]
if not raw_leaderboard:
return🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/masters-tournament/manager.py` around lines 279 - 293, The mock
leaderboard entries use keys pos/name while sort_leaderboard() and
filter_favorite_players() expect position/player, causing mock data to sort as
999 and favorites not to match; fix by normalizing entries returned from
self.data_source.fetch_leaderboard() (map 'pos'->'position' and
'name'->'player', and remove or coerce any invalid positions to an int) before
calling sort_leaderboard() and filter_favorite_players(), leaving the rest of
the logic (favorites, top_n, always_show) intact so both live and mock flows use
the same keys.
masters_hole_by_hole was in the dispatch table and config_key_map but missing from all PHASE_MODES lists, so it could never appear during normal rotation. Added to 5 phases where course content fits: off-season, pre-tournament, practice, tournament-morning, tournament-evening. Left out of tournament-live (too leaderboard-heavy) and overnight/post (not relevant). Mock data key mismatch finding was false - verified mock leaderboard already uses correct position/player keys (not pos/name). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Apr 1, 2026
This was referenced Apr 9, 2026
fix(masters-tournament): wide-short layouts, cache rehydration, flag cleanup, LIVE alert (2.2.1)
#95
Merged
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
What's New
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Assets & Rendering
Data
Configuration
Documentation
Chores