fix(masters-tournament): bug fixes, course tour redesign, dynamic dates, fun_facts config (v2.3.0)#98
Closed
sarjent wants to merge 4 commits intoChuckBuilds:mainfrom
Closed
Conversation
- fun_facts: replace 5-step scroll reset with time-based dwell (20s default, configurable via fun_fact_duration) so long facts have time to scroll fully before advancing to the next one - hole_by_hole: give masters_hole_by_hole its own hole cursor and timer (hole_by_hole_index / last_hole_advance["hole_by_hole"]) independent of masters_course_tour, preventing double-advancement when both modes are active in the same phase rotation - countdown: replace hardcoded April 10 fallback date with MastersDataSource._second_thursday_of_april() so the countdown is correct in future years; also use timezone-aware datetime.now(utc) - live_action: pass leader's actual score-to-par string instead of the literal "Leader" label to render_live_alert, so viewers see the score - score parsing: detect MC/WD/DQ/CUT/MDF ESPN values via _INACTIVE_SCORE_VALUES; add is_active field to player dicts so renderers can distinguish "even par" from "not competing" Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ts config (2.2.9)
course tour (render_hole_card):
- Replaced header+image+footer layout with a true two-column design
across all display tiers (64x32 included):
· Left column: hole number in font_header (larger than detail text),
par and yardage in font_detail, zone badge at bottom when space allows
· Right column: hole map image centred vertically with hole name
centred below it
- Tiny (≤32 wide) falls back to a compact single-column layout
- Updated both MastersRenderer (base) and MastersRendererEnhanced
(_render_hole_card_with_image) to use the new layout
countdown / phase dates:
- Added _masters_thursday(year) to masters_helpers.py implementing the
correct "Thursday between April 6-12" rule, verified against 2022-2026
historical data (fixes _second_thursday_of_april which returned wrong
dates, e.g. April 13 for 2023 when Masters started April 6)
- get_tournament_phase and get_detailed_phase fallback paths now compute
practice/tournament windows dynamically from _masters_thursday instead
of relying on hardcoded April 10-13 day numbers
- masters_data._computed_fallback_meta now delegates to _masters_thursday
- manager._display_countdown hard-fallback uses _masters_thursday directly
fun_facts config:
- _build_enabled_modes now handles bare-boolean mode_config values
("fun_facts": false) in addition to nested {"enabled": false} dicts,
matching what the LEDMatrix web UI may serialise
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- get_detailed_phase: switch from time-based to calendar-day comparisons for the practice/tournament boundary so that early-morning hours on a tournament day (e.g. 6 am Thursday) are not misclassified as "practice" - Fix _masters_thursday docstring: 2028 -> April 6 (not April 12) - Add test_local.py: offline test suite covering Thursday-date logic, get_detailed_phase phase boundaries for 2022-2028, fun_facts config filtering, and visual hole-card rendering at 64x32 / 128x64 / 192x48 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughBumps the Masters Tournament plugin to v2.3.1 and plugins.json last_updated; replaces hardcoded April rules with a computed Masters Thursday window (timezone-aware), refactors mode/config parsing and rotation/timer logic, improves score/is_active handling, redesigns hole-card rendering, and adds a local test script. Changes
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
plugins/masters-tournament/masters_data.py (1)
627-644:⚠️ Potential issue | 🟠 MajorInactive players should not be normalized to
E.Returning
0forMC/WD/DQ/CUT/MDF/--makes downstream renderers showEand count those players in even-par totals, so the newis_activeflag never actually fixes the user-visible bug. This needs a non-score sentinel such asNone, with the sort/render paths handling it explicitly.🤖 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 627 - 644, The _calculate_score_to_par function currently maps inactive score display values (MC/WD/DQ/CUT/MDF/-- and "E" sentinel cases) to 0 which causes downstream renderers to treat them as even par; change the logic in _calculate_score_to_par to return None for any inactive or non-numeric displayValue (use the existing self._INACTIVE_SCORE_VALUES and the checks for empty/"-"/"E" to detect these cases) and only return int for valid numeric scores (handle "+" and "-" prefixes as before); update any callers (sort/render code) to explicitly handle None as “not competing” instead of treating it like 0.
🤖 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/manifest.json`:
- Around line 46-59: Update the new release entries in manifest.json to use the
schema key ledmatrix_min instead of ledmatrix_min_version: in the versions array
(newest entries at the top) replace each "ledmatrix_min_version" property with
"ledmatrix_min" and keep the same values for the existing version, released and
version fields so the manifest conforms to the repo schema.
In `@plugins/masters-tournament/masters_helpers.py`:
- Around line 492-493: The fallback comparison uses timedelta(0) < thu_e - date
which can raise TypeError when callers pass naive datetimes because _to_eastern
leaves them untouched; change the check to compare date objects instead (e.g.,
ensure you compute date_date from the incoming date and use thu_e.date() or
otherwise cast thu_e to a date) so both operands are date objects; update the
logic around thu_e, date and date_date in the relevant function(s) (referencing
_to_eastern, thu_e and date_date) to perform date-to-date subtraction rather
than mixing datetime and date types.
In `@plugins/masters-tournament/test_local.py`:
- Around line 203-206: The test SIZES list is using MastersRenderer for the
"64x32_small" case but manager.py selects MastersRendererEnhanced for width >=
64, so update the SIZES entry for "64x32_small" to use MastersRendererEnhanced
(replace MastersRenderer with MastersRendererEnhanced in that tuple) so the test
exercises the production renderer choice; ensure the tuple remains
("64x32_small", 64, 32, MastersRendererEnhanced).
- Around line 132-134: The current loop registers top-level module stubs
(sys.modules.setdefault(mod_name, MagicMock())) which persist and cause later
imports to return MagicMocks; instead, scope those stubs to the manager import
namespace by first importing the manager module (or using its __name__) and then
registering stubs under f"{manager.__name__}.{mod_name}" (use
sys.modules.setdefault(f"{manager.__name__}.{mod_name}", MagicMock())). Update
the code that currently references sys.modules.setdefault, mod_name and
MagicMock to use the manager-prefixed module names so only imports performed by
the manager get mocked.
---
Outside diff comments:
In `@plugins/masters-tournament/masters_data.py`:
- Around line 627-644: The _calculate_score_to_par function currently maps
inactive score display values (MC/WD/DQ/CUT/MDF/-- and "E" sentinel cases) to 0
which causes downstream renderers to treat them as even par; change the logic in
_calculate_score_to_par to return None for any inactive or non-numeric
displayValue (use the existing self._INACTIVE_SCORE_VALUES and the checks for
empty/"-"/"E" to detect these cases) and only return int for valid numeric
scores (handle "+" and "-" prefixes as before); update any callers (sort/render
code) to explicitly handle None as “not competing” instead of treating it like
0.
🪄 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: dbe9c27f-b399-482b-ac8f-a57bed401ffd
📒 Files selected for processing (8)
plugins.jsonplugins/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/test_local.py
Comment on lines
+46
to
+59
| { | ||
| "version": "2.3.0", | ||
| "released": "2026-04-10", | ||
| "ledmatrix_min_version": "2.0.0" | ||
| }, | ||
| { | ||
| "version": "2.2.9", | ||
| "released": "2026-04-10", | ||
| "ledmatrix_min_version": "2.0.0" | ||
| }, | ||
| { | ||
| "version": "2.2.8", | ||
| "released": "2026-04-10", | ||
| "ledmatrix_min_version": "2.0.0" |
There was a problem hiding this comment.
Use ledmatrix_min in the new release entries.
The newly added versions still use ledmatrix_min_version, but the repo schema for manifest.json version entries is ledmatrix_min. Keeping the old key here can leave compatibility/store tooling reading incomplete metadata for the newest releases.
As per coding guidelines, "Add the new version FIRST (most recent at top) to the versions array in manifest.json, with fields: released (date), version (semver), and ledmatrix_min (minimum LEDMatrix version)".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/masters-tournament/manifest.json` around lines 46 - 59, Update the
new release entries in manifest.json to use the schema key ledmatrix_min instead
of ledmatrix_min_version: in the versions array (newest entries at the top)
replace each "ledmatrix_min_version" property with "ledmatrix_min" and keep the
same values for the existing version, released and version fields so the
manifest conforms to the repo schema.
- get_detailed_phase: replace `thu_e - date` with `thu_date - date_date` (date-to-date subtraction) to avoid TypeError when caller passes a naive datetime that _to_eastern leaves untouched - _calculate_score_to_par: return Optional[int]; None for inactive/unknown (MC/WD/DQ/CUT/MDF/--/empty) instead of 0, so callers can distinguish "not competing" from "even par" - format_score_to_par: accept Optional[int]; return "--" for None - _score_color: return light_gray for None score instead of raising TypeError - sort_leaderboard: sort None-score players to the end (treat as 999) - renderer score reads: drop default=0 so None propagates correctly to format_score_to_par/_score_color; guard field-overview sums with `or 0` - test_local.py SIZES: use MastersRendererEnhanced for 64x32 (matches manager.py which selects Enhanced for width >= 64) - test_local.py stubs: pop masters_renderer/masters_renderer_enhanced from sys.modules before section 4 so real renderer classes are imported Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
Author
|
Closing — will maintain this as a standalone plugin repo instead. |
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
Bug fixes and enhancements for the
masters-tournamentplugin.Bug fixes
fun_fact_durationconfig key, default 20 s) so the fact stays visible for its full duration_hole_by_hole_indexis now independent from_current_hole(used by course tour), preventing the two modes from stomping each other's cursor-12) instead of the static string"Leader"MC,WD,DQ,CUT,MDF,--) are now correctly identified via_INACTIVE_SCORE_VALUES; their score is set toNoneinstead of silently returning0get_detailed_phasemorning boundary: switched from time-based to calendar-day comparison for the practice/tournament boundary so that early-morning hours on tournament days (e.g. 6 am Thursday) are no longer misclassified as"practice"Enhancements
Course tour hole card — two-column layout
Dynamic Masters Thursday date
_masters_thursday(year)helper: finds the Thursday between April 6-12 (the correct rule, verified 2022-2028)_second_thursday_of_april(which returned wrong dates for 2022 and 2023) in all callsites:get_detailed_phase,get_tournament_phase, countdown display, fallback meta computationFun facts config filtering
_build_enabled_modesnow handles bare-boolean mode config values ("fun_facts": false) in addition to the nested dict form ({"enabled": false}), fixing the case where the web UI saves the simpler form and fun facts ignored the settingTesting
test_local.py: an offline test script for Windows/Mac/Linux that requires no Pi, no LED matrix, and no network connection_masters_thursday()date logic for 2022-2028 vs. known historical dataget_detailed_phase()boundary cases across 2023/2024/2026 (practice, morning, live, evening, post, pre, off-season)_build_enabled_modes()fun_facts config filtering (5 scenarios)test_renders/for 64x32, 128x64, 192x48 at holes 1/12/13/18Test plan
python test_local.pyfromplugins/masters-tournament/- all 25 checks should passtest_renders/*.pngto visually inspect two-column hole card layout at each display size"display_modes": {"fun_facts": false}in plugin configGenerated with Claude Code
Summary by CodeRabbit