fix(baseball-scoreboard): stop rendering game start times in UTC - #228
fix(baseball-scoreboard): stop rendering game start times in UTC#228ChuckBuilds wants to merge 2 commits into
Conversation
The plugin looked for the LEDMatrix global timezone only on cache_manager.config_manager. On cores that expose config_manager via the plugin manager instead, that lookup came back empty and every start time was drawn in UTC -- a 6:45pm Central first pitch rendered as 11:45PM -- while clock-simple and geochron, which check plugin_manager first, showed the correct local time on the same device. Timezone resolution now lives in one place (baseball_timezone.py) shared by the switch-mode scorebug, the scroll-mode game card and the plugin manager. Order: plugin override, plugin_manager.config_manager, cache_manager.config_manager, host system zone (TZ, /etc/timezone, /etc/localtime), then UTC. Two related fixes make an explicit override actually usable: - config_schema.json never declared `timezone` and sets additionalProperties: false, so hand-editing the key in the saved config had no effect. It is now a documented string property under Advanced Settings. - The manager assigned self.config["timezone"], mutating the dict the core handed it and persisting a bogus "timezone": "UTC" into the user's saved plugin config. The resolved value is kept on the instance and passed to sub-components via a copy instead. Adds test_timezone_resolution.py covering the resolution order, invalid and blank values, a raising config_manager, and the system-zone backstop. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01QSnspNZceRCdpUtJh2Co6e
|
Warning Review limit reached
Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughBaseball Scoreboard 1.20.0 centralizes timezone resolution, adds an advanced timezone configuration property, updates manager and rendering paths to use resolved timezones without mutating saved configuration, adds regression tests, and documents the release. ChangesBaseball Scoreboard timezone handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Manager
participant TimezoneResolver
participant ConfigManagers
participant Renderer
Manager->>TimezoneResolver: Resolve effective timezone
TimezoneResolver->>ConfigManagers: Check plugin and core configuration
ConfigManagers-->>TimezoneResolver: Return candidate timezone
TimezoneResolver-->>Manager: Return validated timezone
Manager->>Renderer: Pass resolved timezone configuration
Renderer->>TimezoneResolver: Resolve timezone for game start
TimezoneResolver-->>Renderer: Return tzinfo
Renderer->>Renderer: Convert UTC start time to local time
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 48 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/baseball-scoreboard/baseball_timezone.py`:
- Around line 107-118: Update the timezone candidate resolution around the
candidates collection and its consuming loop to evaluate sources lazily,
yielding each candidate only when reached. Preserve the existing priority
order—plugin config, plugin-manager config, cache-manager config, then system
timezone—and stop evaluating lower-priority sources once a valid timezone is
found.
🪄 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 Plus
Run ID: 7a6255e8-da3a-4f12-b821-8e77eee6962a
📒 Files selected for processing (10)
plugins.jsonplugins/baseball-scoreboard/CHANGELOG.mdplugins/baseball-scoreboard/README.mdplugins/baseball-scoreboard/baseball_timezone.pyplugins/baseball-scoreboard/config_schema.jsonplugins/baseball-scoreboard/game_renderer.pyplugins/baseball-scoreboard/manager.pyplugins/baseball-scoreboard/manifest.jsonplugins/baseball-scoreboard/sports.pyplugins/baseball-scoreboard/test_timezone_resolution.py
resolve_timezone_name() built its candidate list eagerly, so every call queried both config managers and stat'd the host timezone files even when the plugin config already supplied a valid zone. SportsCore._get_timezone() runs once per game during extraction, making that a per-game filesystem probe. Yield candidates from a generator instead, preserving the priority order, and assert in the tests that lower-priority sources are left untouched once a candidate resolves. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01QSnspNZceRCdpUtJh2Co6e
2 participants
Pull Request
Summary
The baseball scoreboard rendered every game start time in UTC (a 6:45pm Central first pitch showed as
11:45PM) even on setups where the clock plugin displayed the correct local time. The plugin looked for the LEDMatrix global timezone only oncache_manager.config_manager; on cores that exposeconfig_managervia the plugin manager instead, that lookup came back empty and resolution fell through to UTC. This centralizes timezone resolution, adds the missing fallbacks, and makes thetimezonesetting an actual, documented config option.Type of change
Plugin(s) affected
baseball-scoreboard(1.19.3 → 1.20.0)Related issues
N/A — reported directly.
What changed
New
baseball_timezone.py— single resolver shared by the switch-mode scorebug (sports.py), the scroll-mode game card (game_renderer.py) and the plugin manager. First valid value wins:timezonein the plugin's own config (explicit override)plugin_manager.config_manager— this is the lookup that was missing;clock-simpleandgeochroncheck it first, which is why they were correct on the same devicecache_manager.config_manager(what the plugin used to check exclusively)TZ,/etc/timezone,/etc/localtimeAlso supports older cores that expose
load_config()/get_config()rather thanget_timezone(), skips blank/invalid zone names with a warning instead of adopting them, and never propagates an exception out of a core config manager.Two related fixes make an explicit override actually usable:
config_schema.jsonnever declaredtimezone, and the schema setsadditionalProperties: false— so hand-editing the key in the saved config was discarded. It is now a string property under Advanced Settings, defaulting to""(follow the global timezone).self.config["timezone"] = ...), which persisted a bogus"timezone": "UTC"into the user's saved plugin config. The resolved value now lives on the instance and reaches sub-components through a copy.Test plan
EMULATOR=true python3 run.py)scripts/dev_server.py)New
test_timezone_resolution.py(10 cases, all passing) covers the resolution order, the plugin-level override winning, theplugin_managerlookup that was missing, the legacyload_config()shape, aget_timezone()that raises, blank/invalid values being skipped, the system-zone backstop, the UTC last resort, and an end-to-end conversion of the reported23:45Z → 6:45PMcase.Also run:
python scripts/check_module_collisions.py— OK across 41 plugins (new module is plugin-prefixed per the deferred-import rule)plugins/baseball-scoreboard/test_*.pysuite — every test that passes onmainin this environment still passes.test_baseball_plugin.py,test_config_reload.py,test_score_antialiasing.pyandtest_test_mode_live_games.pyfail identically before and after this change (they need the coresrcpackage and font assets that aren't present here).Not verified on hardware or in the emulator — worth a sanity check on a real panel before release.
Required for plugin changes
versioninplugins/<id>/manifest.json(1.19.3 → 1.20.0)class_nameinmanifest.jsonmatches the actual class inmanager.pyexactly (unchanged)entry_pointmatches the real file (unchanged)README.mdif config keys changed — documentstimezoneplus a troubleshooting entry for the UTC symptomconfig_schema.jsonis the source of truth for the web UI form —timezoneadded withdefault,descriptionandx-advancedplugins.json)Checklist
CONTRIBUTING.mdCONTRIBUTING.mdandCODE_OF_CONDUCT.mdNotes for reviewer
Existing installs need one manual step. Anyone who ran an affected version already has
"timezone": "UTC"written into their saved baseball config by the old write-back bug. Since an explicit plugin-level value wins, that stale"UTC"will keep overriding the fix until it's cleared or set to the right zone. It's now editable in the web UI under Advanced Settings, and the README troubleshooting entry calls it out. I deliberately avoided auto-healing a stored"UTC"— there's no way to tell it apart from a deliberate choice, and silently overriding it would break users who genuinely want UTC.sports.pycalls_get_timezone()once per game during extraction. In the normal path the resolver returns on the first candidate (the manager already injected the resolved name into the manager config), so no filesystem probing happens per game;pytz.timezone()is internally memoized.Generated by Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation