fix(calendar): keep yearly events in DTSTART's month when BYMONTH is missing

[pascalpfammatter]

A birthday in my Google Calendar showed up on my mirror as "tomorrow" when it is actually in October. It turns out it had been appearing on the 2nd of every month, and I had simply never noticed until it landed on a day I paid attention to.

The event looks like this in the ICS feed:

DTSTART;VALUE=DATE:20231002
DTEND;VALUE=DATE:20231003
RRULE:FREQ=YEARLY;WKST=MO;INTERVAL=1;BYMONTHDAY=2
SUMMARY:Ted Birthday

FREQ=YEARLY together with BYMONTHDAY=2, but no BYMONTH. BYMONTHDAY is an expanding rule part for FREQ=YEARLY, so the expander returns the 2nd of every month — twelve occurrences a year instead of one. Google Calendar's own UI, and the client that wrote the rule, show it once a year on 2 October.

Out of 3354 events in my calendar exactly two had this shape, both created by an older mobile calendar app. Newer clients write a plain RRULE:FREQ=YEARLY, which is why this is easy to miss — but the old events are never rewritten, so they keep misbehaving forever.

Why not fix it in the RRULE library

This has come up before, and each time it was closed as bad input data rather than fixed: #2547 (2021), #3047 (2023), and recently jens-maus/node-ical#531. In that last one a fix was actually written (ggaabe/rrule-temporal#127) and then withdrawn, because a reviewer pointed out that restricting yearly BYMONTHDAY rules to the DTSTART month breaks legitimate rules such as FREQ=YEARLY;BYMONTHDAY=1,3, which really must expand across all twelve months.

That objection is correct, and it is why I don't think the RRULE engine is the right place for this. A general-purpose expander has to follow RFC 5545. A calendar display, on the other hand, can afford to be liberal about a well-known broken export as long as the detection is narrow enough that no correct rule is touched.

What this does

expandRecurringEvent confines a yearly rule to DTSTART's month, but only when the rule is provably a redundant restatement of DTSTART rather than a real expansion. All of these must hold:

• FREQ=YEARLY
• BYMONTHDAY holds exactly one value
• that value equals DTSTART's day-of-month
• no BYMONTH, BYDAY, BYYEARDAY, BYWEEKNO or BYSETPOS

Under those conditions the expansion yields DTSTART's own date plus eleven dates the author never wrote, so dropping the extras cannot lose a real occurrence.

Rule	Affected?	Result
FREQ=YEARLY;...;BYMONTHDAY=2, DTSTART Oct 2	yes	once a year, 2 October
FREQ=YEARLY;BYMONTHDAY=1,3	no, two values	unchanged, expands over 12 months
FREQ=YEARLY;BYMONTHDAY=13;BYDAY=FR	no, BYDAY present	unchanged, every Friday the 13th
FREQ=YEARLY;BYMONTHDAY=7, DTSTART on the 2nd	no, day mismatch	unchanged, expands
FREQ=YEARLY;BYMONTHDAY=2;BYMONTH=10	no, BYMONTH present	unchanged, once a year
FREQ=YEARLY	no, no BYMONTHDAY	unchanged, once a year

The rule is read through node-ical's public options, and both the current string freq and the older numeric one are accepted.

Tests

Six new cases in calendar_fetcher_utils_spec.js. The five "unaffected" rows above are tests that already pass without the change — they are there to pin the behaviour that must not regress. Only the first one fails before the fix.

Calendar unit tests pass on Europe/Zurich, America/New_York, Pacific/Auckland, Asia/Kolkata and UTC. lint:js and lint:prettier are clean.

Against my own feed over the next 365 days the number of occurrences goes from 132 to 110, and the difference is exactly the 22 spurious instances from those two events — nothing else changes.

One thing worth deciding

I've made this unconditional, since the detection is narrow enough that no correct rule is affected. If you would rather have it behind a calendar config option, say so and I'll move it.

[KristjanESPERANTO]

Thanks for the deep dive here, and for the references to the earlier discussions. I think the direction here is fine.

What do you think of this: instead of reading rrule.options and guessing between camelCase/lowercase keys and string/numeric freq, read rrule.origOptions. It only contains what was actually written in the RRULE (not what the library fills in as a default), so detecting "no BYMONTH" becomes unambiguous.

	isYearlyRuleMissingByMonth (event) {
		const options = event.rrule?.origOptions;
		if (!options || options.freq !== "YEARLY") {
			return false;
		}

		const isSingleMonthDay = Array.isArray(options.byMonthDay) && options.byMonthDay.length === 1;
		// Any other BYxxx part means the rule shapes the recurrence on purpose.
		const hasShapingPart = Boolean(options.byMonth || options.byDay || options.byYearDay || options.byWeekNo || options.bySetPos);
		if (!isSingleMonthDay || hasShapingPart) {
			return false;
		}

		return options.byMonthDay[0] === event.start.getDate();
	},

[KristjanESPERANTO]

@pascalpfammatter Did you noticed my comment? 🙂

[KristjanESPERANTO]

Since we didn't receive any feedback, I went ahead and made the change I suggested myself.