Skip to content

fix(weather): recover missing moonset and show true moon illumination on the almanac page - #157

Closed
rpierce99 wants to merge 3 commits into
ChuckBuilds:mainfrom
rpierce99:fix/almanac-moon-data
Closed

fix(weather): recover missing moonset and show true moon illumination on the almanac page#157
rpierce99 wants to merge 3 commits into
ChuckBuilds:mainfrom
rpierce99:fix/almanac-moon-data

Conversation

@rpierce99

@rpierce99 rpierce99 commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

What

Two fixes to the weather plugin's almanac (moon) page.

1. Missing moonset (---)

Once per lunar month the moonset slot rendered ---. astral's moonset() only searches within a single calendar day, so on the day the moon's set straddles local midnight it raises "Moon never sets on this date, at this location" even though the moon plainly sets that afternoon.

This is a known, open, unreleased astral bug — sffjunkie/astral#88 (2023) and #105 (2026) — and 3.2 is the latest release on PyPI, so there's no upgrade to wait for.

Real example — Federal Way, WA on 2026-06-11: moon rises 02:24, sets ~17:23, but astral.moonset() raises. The fix catches the failure and recovers the time by scanning the moon's elevation across the local day (using astral's own moon.elevation() ephemeris) and bisecting the horizon crossing. No new dependencies, no orbital math reimplemented. The same fallback covers moonrise.

It agrees with astral to the minute on normal days (verified against 2026-06-12: rise 02:48 vs astral 02:49, set 18:50 vs 18:50), so it only ever changes the broken days.

2. "Illumination %" was cycle progress, not lit fraction

The percentage next to the phase name was int(moon_phase * 100), but moon_phase is progress through the cycle (0=new, 0.5=full, 0.75=last quarter), not the illuminated fraction. So a waning crescent read "86%" when it's ~13% lit. They only agree at the quarters. Now converted via (1 − cos(2π·phase)) / 2.

Tests

  • New test_almanac_moon_data.py — fails before, passes after. Covers the 2026-06-11 moonset astral can't compute, fallback/astral agreement on a normal day, and the illumination conversion (incl. that a crescent no longer reads as its cycle fraction).
  • Existing test_almanac_layout.py and test_geocode_cache.py still pass.
  • Plugin safety harness (check_plugin.py) passes for ledmatrix-weather at every panel size.

Version

Patch bump 2.5.0 → 2.5.1; plugins.json, manifest versions[], and CHANGELOG updated.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Restored missing moonset times when events span midnight.
    • Corrected illumination percentage to show actual lit fraction.
  • Tests

    • Added regression tests for moonrise/moonset fallback and illumination calculations.
  • Chores

    • Plugin updated to v2.5.1 and metadata timestamps advanced.

The almanac (moon) page had two defects:

1. Once a month the moonset slot showed "---". astral only searches a
   single calendar day, so on the day the moon's set straddles midnight
   it raises "Moon never sets on this date" even though the moon sets
   that afternoon (sffjunkie/astral ChuckBuilds#88, ChuckBuilds#105, both open as of astral
   3.2, the latest release). When astral fails, fall back to scanning
   the moon's elevation across the day and bisecting the horizon
   crossing with astral's own ephemeris. Matches astral to the minute on
   normal days; the same fallback covers moonrise.

2. The "illumination %" was actually the cycle fraction, so a waning
   crescent read 86% instead of ~13% lit. Convert phase to true
   illuminated fraction.

Adds test_almanac_moon_data.py (fails before, passes after) covering the
2026-06-11 Federal Way moonset astral can't compute and the illumination
conversion.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 70d677be-a21e-4c3a-ba0d-9ad3687f3d71

📥 Commits

Reviewing files that changed from the base of the PR and between 206a4fd and 2ea976d.

📒 Files selected for processing (4)
  • plugins.json
  • plugins/ledmatrix-weather/manager.py
  • plugins/ledmatrix-weather/manifest.json
  • plugins/ledmatrix-weather/test_almanac_moon_data.py
🚧 Files skipped from review as they are similar to previous changes (3)
  • plugins.json
  • plugins/ledmatrix-weather/manifest.json
  • plugins/ledmatrix-weather/test_almanac_moon_data.py

📝 Walkthrough

Walkthrough

Weather plugin v2.5.1 fixes moon illumination (now cosine-based percent) and adds a moonrise/moonset fallback when Astral fails for events that straddle midnight; regression tests and manifest/registry metadata are updated.

Changes

Moon Data Fixes

Layer / File(s) Summary
Moon illumination and rise/set fallback implementation
plugins/ledmatrix-weather/manager.py
Added math import; introduced _moon_illumination() to compute true illuminated fraction via cosine; added _astral_moon_event() and _moon_event_fallback() to recover rise/set by scanning elevation and bisecting horizon crossings; integrated fallback into _map_om_daily() and updated almanac illumination formatting.
Moon data regression test suite
plugins/ledmatrix-weather/test_almanac_moon_data.py
Added regression test module with timezone/observer constants and checks: moonset fallback recovery on 2026-06-11 (Astral edge case), fallback consistency within 90s of Astral on normal dates, illumination correctness at reference phases (including crescent), and a runnable PASS/FAIL main().
Version bump and changelog documentation
plugins/ledmatrix-weather/manifest.json, plugins/ledmatrix-weather/CHANGELOG.md, plugins.json
Bumped manifest version 2.5.0 → 2.5.1; added 2.5.1 release entry with moon/almanac fixes; updated last_updated timestamps and latest_version in plugins.json; added CHANGELOG entry for 2.5.1.

Possibly Related PRs

  • ChuckBuilds/ledmatrix-plugins#126: Introduced earlier Astral-based moon/almanac logic in manager.py; this PR adds failure recovery and illumination corrections on that foundation.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately and specifically describes the main changes: fixing missing moonset recovery and correcting moon illumination display on the almanac page.
Docstring Coverage ✅ Passed Docstring coverage is 81.82% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codacy-production

codacy-production Bot commented Jun 12, 2026

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 29 complexity

Metric Results
Complexity 29

View in Codacy

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 coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
plugins/ledmatrix-weather/manifest.json (1)

4-32: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Release-date metadata is only half-updated in plugins/ledmatrix-weather/manifest.json and plugins.json. The new 2.5.1 release is dated 2026-06-11, but plugins/ledmatrix-weather/manifest.json still leaves last_updated at 2026-06-10, and plugins.json still leaves the weather entry’s last_updated at 2026-06-10. Updating both fields together avoids stale cache/freshness signals for the new weather release.

🤖 Prompt for 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.

In `@plugins/ledmatrix-weather/manifest.json` around lines 4 - 32, Update the
release-date metadata by setting the JSON key "last_updated" to "2026-06-11" for
both the plugin manifest and the global plugins listing: edit the plugin's
manifest.json entry that contains "version": "2.5.1" and change its
"last_updated" value, and also update the corresponding entry in plugins.json
for the weather plugin so its "last_updated" matches the new release date;
ensure the date strings match exactly "2026-06-11".
🤖 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/ledmatrix-weather/manager.py`:
- Around line 738-750: The try/except around astral_moon.moonrise() and
astral_moon.moonset currently swallows all exceptions; change it to only catch
ValueError that matches Astral’s “Moon never rises on this date, at this
location” / “Moon never sets on this date, at this location” messages and call
self._moon_event_fallback(...) in that case, but re-raise any other exception.
Locate the moonrise and moonset calls in the block that computes
moonrise_dt/moonset_dt (the methods invoking astral_moon.moonrise and
astral_moon.moonset) and replace the broad except Exception with logic that
checks isinstance(e, ValueError) and the error string before using the fallback,
otherwise raise the exception.

In `@plugins/ledmatrix-weather/test_almanac_moon_data.py`:
- Around line 46-49: Replace the broad except Exception around the
astral_moon.moonset(OBS, d, tzinfo=TZ) call with an except ValueError that only
swallows the specific "Moon never sets on this date, at this location" condition
(e.g., except ValueError as exc and assert or check that "Moon never sets" is in
str(exc)); leave the other except Exception as exc blocks (lines 73-77)
unchanged so unexpected errors still fail the test. This targets the
astral_moon.moonset failure case and avoids hiding unrelated exceptions while
preserving the astral_raised boolean behavior.

---

Outside diff comments:
In `@plugins/ledmatrix-weather/manifest.json`:
- Around line 4-32: Update the release-date metadata by setting the JSON key
"last_updated" to "2026-06-11" for both the plugin manifest and the global
plugins listing: edit the plugin's manifest.json entry that contains "version":
"2.5.1" and change its "last_updated" value, and also update the corresponding
entry in plugins.json for the weather plugin so its "last_updated" matches the
new release date; ensure the date strings match exactly "2026-06-11".
🪄 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: 8f776f21-fdf9-4cc8-a4d0-9669497ccfa9

📥 Commits

Reviewing files that changed from the base of the PR and between 7d3a91e and 206a4fd.

📒 Files selected for processing (5)
  • plugins.json
  • plugins/ledmatrix-weather/CHANGELOG.md
  • plugins/ledmatrix-weather/manager.py
  • plugins/ledmatrix-weather/manifest.json
  • plugins/ledmatrix-weather/test_almanac_moon_data.py

Comment thread plugins/ledmatrix-weather/manager.py Outdated
Comment thread plugins/ledmatrix-weather/test_almanac_moon_data.py Outdated
rpierce99 and others added 2 commits June 11, 2026 21:11
Address PR review: the blind `except Exception` around astral's
moonrise/moonset could mask unrelated errors and silently render the
fallback. Catch astral's documented `ValueError` ("Moon never rises/sets
on this date") for the fallback, keep its inconsistent `None` return as
the same recovery path, and log any other unexpected error while
reporting that one field as missing.

Deliberately do NOT re-raise unexpected errors as the reviewer suggested:
update() wraps the whole fetch in an except-Exception backoff, so
propagating would blank the entire weather widget over a secondary moon
field. Refactored the duplicated moonrise/moonset wiring into
_astral_moon_event(). Test guard narrowed to the specific ValueError.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Address PR review: the 2.5.1 versions[] entry is dated 2026-06-11 but
both last_updated fields (manifest.json and the plugins.json weather
entry) were left at 2026-06-10, giving stale freshness signals for the
new release. update_registry.py only syncs last_updated on a version
increase, so re-running after the earlier bump didn't catch it; set both
to 2026-06-11 to match the release.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@rpierce99

Copy link
Copy Markdown
Contributor Author

Addressed the last_updated nitpick in 2ea976d — set both manifest.json and the plugins.json weather entry to 2026-06-11 to match the 2.5.1 release date. (Root cause: update_registry.py only syncs last_updated on a version increase, so re-running it after the version was already bumped didn't refresh the date — worth moving last_updated into its always-synced metadata loop in a separate change.)

@rpierce99

Copy link
Copy Markdown
Contributor Author

Superseded by #163, which combines this with #159 into a single 2.5.1 release (both PRs bumped to 2.5.1, so they couldn't both land).

@rpierce99 rpierce99 closed this Jun 14, 2026
ChuckBuilds pushed a commit that referenced this pull request Jun 15, 2026
…mes (2.5.1) (#163)

Combines the two open almanac (moon) page PRs into a single 2.5.1 release so
they don't both claim the same version. Supersedes #157 and #159.

- Recover missing moonset: astral only searches a single calendar day, so once
  a month the event straddles midnight and it raises "Moon never sets on this
  date" (sffjunkie/astral #88, #105). Fall back to scanning the moon's
  elevation across the day and bisecting the horizon crossing. Same fallback
  covers moonrise.
- Show true illumination: the "%" next to the phase name showed cycle progress
  (so a waning crescent read 86% instead of ~13% lit); it now converts phase to
  the true illuminated fraction.
- Restore full moon-phase names ("Waxing/Waning Crescent", "Waxing/Waning
  Gibbous") now that the redesigned layout has room: full name where it fits,
  abbreviation when tight, character trim only as a last resort.

Tests: test_almanac_moon_data.py, extended test_almanac_layout.py (incl. the
full-name assertions), and test_geocode_cache.py all pass. Almanac golden
images regenerated for the combined output and the plugin safety harness passes
at every supported size.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant