fix(masters): countdown priority, URL cache hygiene, narrow exceptions (v2.4.3)#104
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughReleased version 2.4.3 of the masters-tournament plugin with metadata updates. Changes include refined exception handling for image processing, added microsecond normalization to computed fallback timestamps, and increased off-season countdown mode frequency in rotation. Changes
Possibly related PRs
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
plugins/masters-tournament/manager.py (1)
173-184: Trailingmasters_countdownat line 183 is now redundant.With the two new top-of-list entries (lines 174–175),
masters_countdownnow appears 3 times in the off-season rotation. Per the PR description ("move … to the top … and repeat it"), the pre-existing trailing entry on line 183 looks like a leftover from the prior ordering. If the intent is ~2/8 dominance, drop the trailing one; if 3/9 is deliberate, a brief comment would help future readers.Suggested tidy-up (if the trailing one is leftover)
"masters_course_overview", "masters_tournament_stats", - "masters_countdown", ],🤖 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 173 - 184, The "off-season" rotation array contains "masters_countdown" three times (two consecutive entries at the top plus a trailing one) which appears redundant; edit the off-season list in manager.py to remove the trailing "masters_countdown" entry (or, if you intended three occurrences, add a brief comment next to the array to explain the 3/9 weighting). Specifically update the "off-season" array where "masters_countdown" is repeated so the final rotation reflects the intended frequency (either remove the last "masters_countdown" or document the deliberate triple occurrence).plugins/masters-tournament/masters_data.py (1)
280-287: Midnight-EDT normalization is correct; consider preserving microseconds.Math checks out:
_masters_thursday()returns 12:00 UTC;replace(hour=4, minute=0, second=0)yields 04:00 UTC = 00:00 EDT on Thursday, and+ 3d23h59m59slands on Sunday 23:59:59 EDT — consistent with_parse_tournament_meta's padded end_date.Minor nit:
replace(hour=4, minute=0, second=0)does not resetmicrosecond._masters_thursdayconstructs the datetime with no microsecond argument so it's 0 in practice, but explicitly zeroing it (microsecond=0) would make this robust to future changes to that helper.Optional hardening
- thu_midnight_edt = start.replace(hour=4, minute=0, second=0) + thu_midnight_edt = start.replace(hour=4, minute=0, second=0, microsecond=0)🤖 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 280 - 287, The replace call that normalizes `_masters_thursday()` to EDT uses replace(hour=4, minute=0, second=0) but doesn't clear microseconds; update the normalization so `thu_midnight_edt = start.replace(hour=4, minute=0, second=0, microsecond=0)` (affecting the variable `thu_midnight_edt` in this block) to ensure microseconds are explicitly zeroed and keep behavior consistent with `_parse_tournament_meta` and future changes to `_masters_thursday`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@plugins/masters-tournament/manager.py`:
- Around line 173-184: The "off-season" rotation array contains
"masters_countdown" three times (two consecutive entries at the top plus a
trailing one) which appears redundant; edit the off-season list in manager.py to
remove the trailing "masters_countdown" entry (or, if you intended three
occurrences, add a brief comment next to the array to explain the 3/9
weighting). Specifically update the "off-season" array where "masters_countdown"
is repeated so the final rotation reflects the intended frequency (either remove
the last "masters_countdown" or document the deliberate triple occurrence).
In `@plugins/masters-tournament/masters_data.py`:
- Around line 280-287: The replace call that normalizes `_masters_thursday()` to
EDT uses replace(hour=4, minute=0, second=0) but doesn't clear microseconds;
update the normalization so `thu_midnight_edt = start.replace(hour=4, minute=0,
second=0, microsecond=0)` (affecting the variable `thu_midnight_edt` in this
block) to ensure microseconds are explicitly zeroed and keep behavior consistent
with `_parse_tournament_meta` and future changes to `_masters_thursday`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5be00441-0f8b-4711-87db-8a838192bf4c
📒 Files selected for processing (6)
plugins.jsonplugins/masters-tournament/logo_loader.pyplugins/masters-tournament/manager.pyplugins/masters-tournament/manifest.jsonplugins/masters-tournament/masters_data.pyplugins/masters-tournament/masters_helpers.py
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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`:
- Line 174: The inline comment containing "3×" still uses the non-ASCII
multiplication sign and triggers Ruff RUF003; replace the Unicode '×' with the
ASCII 'x' (i.e., change "3×" to "3x") in the comment near the masters_countdown
reference (search for the comment containing "masters_countdown" or the exact
"3× out of 10 entries" text) so the PR matches the stated intent.
🪄 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: 60c23fa3-57d6-4d4e-a440-353b1ca1af6a
📒 Files selected for processing (2)
plugins/masters-tournament/manager.pyplugins/masters-tournament/masters_data.py
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/masters-tournament/masters_data.py
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
Replace catch-all Exception with specific types — RequestException, UnidentifiedImageError, OSError — so only expected HTTP/image/file errors are masked; add missing imports for requests.exceptions and PIL.UnidentifiedImageError. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Move masters_countdown to the top of the off-season PHASE_MODES list and repeat it so it gets dominant screen time after the post-tournament window closes, matching user expectation that "the countdown kicks in" after tournament results are no longer displayed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add comment to off-season PHASE_MODES explaining the deliberate 3/10 countdown frequency (~30% screen time) - Zero microseconds in _computed_fallback_meta thu_midnight_edt normalization for consistency Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
97693e4 to
ff39c38
Compare
Summary
masters_countdownwas the last entry in the off-season mode list, giving it the least screen time. Moved it to the top and repeated it so it dominates after the post-tournament window closes — matching the expected behavior that "the countdown kicks in" once tournament results are no longer shown.player_idis absent, the cache key previously used the raw URL (high cardinality, leaks query params in logs). Now uses a 12-char SHA-256 prefix as a stable key. Disk filename logic forplayer_idis unchanged.except Exceptionin headshot download: Replace catch-all with(requests.exceptions.RequestException, UnidentifiedImageError, OSError)so only expected HTTP/image/file errors are masked. Adds required imports forrequests.exceptionsandPIL.UnidentifiedImageError.×multiplication sign withxin inline comment to satisfy Ruff RUF003.Test plan
player_idis absentWARNINGwith the hashed key (not the raw URL)🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Chores