fix(api): expand recurring events when serving range queries#382
Merged
Conversation
`getEventsInRange` was matching events with `eventStatesTable.startTime BETWEEN start AND end`. For recurring events, `startTime` holds the first occurrence only, so DAILY/WEEKLY/etc. masters whose first occurrence falls outside the window never matched — every later occurrence was lost. This made the ICS provider effectively unusable as a source of truth for any feed with recurring events. Materialise occurrences at query time using `ts-ics`'s `extendByRecurrenceRule`, honouring EXDATE and the rule's `until` / `count` terminators. The synced query now selects: - one-off rows whose `startTime` is in-window, OR - recurring masters whose `startTime <= windowEnd` and the in-app expansion step filters each materialised occurrence to [start, end]. The master's duration is preserved on each occurrence. Each materialised occurrence gets a synthetic id of the form `<masterId>_<occurrenceISO>` so downstream consumers can distinguish instances. RECURRENCE-ID overrides (per-instance edits beyond plain EXDATE) are not yet persisted by the ingest path and so are not honoured here — that requires a separate schema change. Exports `parseRecurrenceRuleFromJson` / `parseExceptionDatesFromJson` from `@keeper.sh/calendar` so the API can reuse them. Adds unit tests covering DAILY/WEEKLY rules, EXDATE handling, UNTIL/COUNT terminators, out-of-window masters, and synthetic-id stability.
ridafkih
approved these changes
May 23, 2026
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
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
getEventsInRangematches event rows with a flateventStatesTable.startTime BETWEEN start AND endfilter. For recurring events the master row'sstartTimeholds only the first occurrence, so DAILY / WEEKLY / etc. masters whose first occurrence falls outside the queried window are dropped entirely — every later occurrence is lost. This makes the ICS provider effectively unusable as a source of truth for any feed with recurring events (Bankingly feed in my case, but the same applies to anything pulled from Outlook / Exchange / Google ICS exports).Repro against a fresh keeper instance synced with an ICS feed that has a DAILY event:
returns only the one-off VEVENTs scheduled that day. The daily recurring meeting that should land at 10:00 is missing because its master
startTimeis two months earlier.Why
ICS ingest in
packages/calendar/src/ics/utils/parse-ics-events.tscorrectly readsrecurrenceRuleandexceptionDatesand stores them on the master row inevent_states. The destination/write path consumes them viaextendByRecurrenceRuleinhasActiveFutureOccurrenceto decide whether a recurring master is still active.The read path never materialises occurrences.
getEventsInRangeonly querieseventStatesTable.startTime BETWEEN start AND end, so the master row is included iff its first occurrence is in-window. If the rule has run for months, every subsequent occurrence is missing from the response.This is symmetric with what the destination path already does (it knows recurring masters are special) — the read path just hadn't caught up.
Changes
Two source files + one new test file.
services/api/src/queries/expand-recurring-event.ts(new)Pure function that takes a stored master row and the
[windowStart, windowEnd]instants and returns the occurrences that fall in the window:Each occurrence gets a synthetic id of the form
${masterId}_${occurrenceISO}so downstream consumers can distinguish instances. The master's duration is carried onto each occurrence.services/api/src/queries/get-events-in-range.tsThe synced-events query now considers two row classes:
Non-recurring rows still match by
startTimein-range. Recurring masters match when their first occurrence is at-or-before the window end (later occurrences may still land inside the window even if the master is far in the past); the in-app expansion step then filters each materialised occurrence to[start, end].packages/calendar/src/index.tsExports
parseRecurrenceRuleFromJsonandparseExceptionDatesFromJsonso the API service can reuse them without reaching into internal paths.userEventsTabledoesn't haverecurrenceRule/exceptionDatescolumns, so the user-events query is unchanged.Testing
Added
services/api/tests/queries/expand-recurring-event.test.tscovering:untilterminator caps the expansioncountterminator caps the expansionbun run typesclean acrossservices/apiandpackages/calendar.Manual verification against a Bankingly ICS feed: the previously-missing
Daily HNrecurring event now shows up inget_eventsresponses for every day in the queried window; EXDATE'd days are correctly skipped.Compatibility
KeeperEvent.idis typed asstring, which the synthetic id format respects. Web clients that store ids as React keys / map entries keep working — keys are stable per occurrence.recurrenceRule(the existingstartTime BETWEEN start AND endfilter).Out of scope
recurrence_overridestable keyed by(master_id, occurrenceISO)— and worth its own PR once the storage shape is decided. For the common case of plain RRULE + EXDATE, the current PR is sufficient.get_event(synthetic_id). The MCPget_eventtool still expects a real master UUID and won't dereference a synthetic occurrence id. The expansion only affectsget_events(list); fetching a recurring instance by its synthetic id would require a separate routing change. Mentioned for completeness but I don't see a current consumer that needs it.getEventsInRangedoesn't paginate today; expanding recurring events makes large windows (e.g. a year of dailies) potentially noisy. The expansion is bounded bywindowEnd, so as long as callers use reasonable windows (the web frontend uses 7-day pages) this is fine. If pagination is added later, the expansion will need to be re-considered to keep page boundaries deterministic.