M1: Wire RRULE recurrence expansion into calendar event queries #215

2026-07-29T22:28:28Z

KofTwentyTwo commented

2026-07-29 22:28:28 +00:00

(Migrated from github.com)

Closes #186.

Summary

Added DatabaseEngine::list_recurring_calendar_events(account_id) — returns every persisted event with a non-empty rrule for the account, with no time-window filter (a recurring master's own stored timestamps can sit outside a query window while its occurrences fall inside it).
Added a pure, directly-unit-testable merge helper expand_events_for_window(all_in_window, recurring, start_window, end_window) in crates/nunciod/src/grpc.rs: keeps non-recurring rows from all_in_window verbatim, and for every event in recurring calls nuncio_cal::RecurrenceEngine::expand_occurrences to materialize real occurrences inside the window. Recurring rows are always sourced from recurring/expansion, never from all_in_window, so a recurring master that also happens to overlap the window in the raw query is never double-counted.
Wired the helper into CalendarGrpcService::list_events, which now fetches both list_calendar_events and list_recurring_calendar_events and merges them.
An rrule that fails to parse degrades to the raw stored master with tracing::warn!, mirroring the pattern in FiltersGrpcService::reload_engine_from_store — a single bad rule never fails the whole ListEvents call.
Applied the previously-ignored calendar_id filter (deferred scope from the M1-S1 review of #185): list_events now retains only events matching req.calendar_id when it's non-empty.
Fixed a genuine correctness/safety gap in RecurrenceEngine::expand_occurrences while wiring it in: it previously fell back to returning the raw, potentially out-of-window master event whenever zero occurrences matched the window (e.g. a window entirely past an UNTIL/COUNT bound), which would have fabricated a result outside the requested window once this was wired into list_events. Also, filtering before take() on an unbounded (no UNTIL/COUNT) series whose occurrences never enter a past/pre-series window could scan forever. Replaced with skip_while/take_while over the RRuleSet's monotonically increasing iterator, which bounds the walk and returns a genuinely empty result when nothing matches.

How recurrence expansion is wired

list_events now does:

let all_in_window = db.list_calendar_events(&req.account_id, req.start_window, req.end_window).await?;
let recurring = db.list_recurring_calendar_events(&req.account_id).await?;
let mut events = expand_events_for_window(all_in_window, recurring, req.start_window, req.end_window);
events.retain(|e| req.calendar_id.is_empty() || e.calendar_id == req.calendar_id);

An out-of-window master with in-window occurrences is handled by never filtering recurring by the window at the store layer — every recurring master for the account is always fetched and always passed through expand_occurrences, which does the real per-occurrence windowing.

Tests added

nuncio-store: list_recurring_calendar_events_filters_by_account_and_rrule_presence (rrule-null exclusion, account scoping).
nuncio-cal: recurring_event_with_no_occurrences_in_window_returns_empty, until_bound_stops_occurrences_past_the_bound (the engine-level correctness fix above).
nunciod::grpc: pure unit tests on expand_events_for_window — later-occurrence-of-a-recurring-event found, non-recurring passthrough unchanged, no double-counting a master that also overlaps the window, UNTIL bound honored, unparseable rrule degrades to the raw master. Plus an RPC-level test list_events_excludes_events_from_a_different_calendar_id for the calendar_id filter.

Gate evidence

cargo fmt --all -- --check   -> clean
cargo check-all               -> clean (clippy --all-targets --workspace -D warnings, 0 warnings)
cargo test-all                -> 0 failed, 0 ignored across the workspace
  nuncio-cal:  25 passed
  nuncio-store: 51 passed
  nunciod:      91 passed (unit) + calendar_e2e_test 1 passed + daemon_e2e_test 1 + grpc_auth_test 3 + spine_e2e_test 1
  (all other workspace crates unaffected, all green)

Pre-commit hook (fmt/clippy/test-all) also ran and passed on the commit.

Decisions / notes for PO review

No proto change — ListEventsResponse.events already carries the expanded occurrences; not a [PROTO] story.
nuncio-store gained no dependency on nuncio-cal; expansion happens entirely at the nunciod gRPC layer, per the issue's layering guidance.
The RecurrenceEngine::expand_occurrences behavior change (removing the "return raw master on empty match" fallback, switching to skip_while/take_while) is a real engine-level fix, not just RPC wiring — flagging it explicitly since the issue said "no change expected" unless a genuine gap was found. Happy to discuss if a different tradeoff is preferred.

Closes #186. ## Summary - Added `DatabaseEngine::list_recurring_calendar_events(account_id)` — returns every persisted event with a non-empty `rrule` for the account, with **no** time-window filter (a recurring master's own stored timestamps can sit outside a query window while its occurrences fall inside it). - Added a pure, directly-unit-testable merge helper `expand_events_for_window(all_in_window, recurring, start_window, end_window)` in `crates/nunciod/src/grpc.rs`: keeps non-recurring rows from `all_in_window` verbatim, and for every event in `recurring` calls `nuncio_cal::RecurrenceEngine::expand_occurrences` to materialize real occurrences inside the window. Recurring rows are always sourced from `recurring`/expansion, never from `all_in_window`, so a recurring master that also happens to overlap the window in the raw query is never double-counted. - Wired the helper into `CalendarGrpcService::list_events`, which now fetches both `list_calendar_events` and `list_recurring_calendar_events` and merges them. - An `rrule` that fails to parse degrades to the raw stored master with `tracing::warn!`, mirroring the pattern in `FiltersGrpcService::reload_engine_from_store` — a single bad rule never fails the whole `ListEvents` call. - Applied the previously-ignored `calendar_id` filter (deferred scope from the M1-S1 review of #185): `list_events` now retains only events matching `req.calendar_id` when it's non-empty. - **Fixed a genuine correctness/safety gap in `RecurrenceEngine::expand_occurrences`** while wiring it in: it previously fell back to returning the raw, potentially out-of-window master event whenever zero occurrences matched the window (e.g. a window entirely past an `UNTIL`/`COUNT` bound), which would have fabricated a result outside the requested window once this was wired into `list_events`. Also, filtering before `take()` on an unbounded (no `UNTIL`/`COUNT`) series whose occurrences never enter a past/pre-series window could scan forever. Replaced with `skip_while`/`take_while` over the `RRuleSet`'s monotonically increasing iterator, which bounds the walk and returns a genuinely empty result when nothing matches. ## How recurrence expansion is wired `list_events` now does: ```rust let all_in_window = db.list_calendar_events(&req.account_id, req.start_window, req.end_window).await?; let recurring = db.list_recurring_calendar_events(&req.account_id).await?; let mut events = expand_events_for_window(all_in_window, recurring, req.start_window, req.end_window); events.retain(|e| req.calendar_id.is_empty() || e.calendar_id == req.calendar_id); ``` An out-of-window master with in-window occurrences is handled by never filtering `recurring` by the window at the store layer — every recurring master for the account is always fetched and always passed through `expand_occurrences`, which does the real per-occurrence windowing. ## Tests added - `nuncio-store`: `list_recurring_calendar_events_filters_by_account_and_rrule_presence` (rrule-null exclusion, account scoping). - `nuncio-cal`: `recurring_event_with_no_occurrences_in_window_returns_empty`, `until_bound_stops_occurrences_past_the_bound` (the engine-level correctness fix above). - `nunciod::grpc`: pure unit tests on `expand_events_for_window` — later-occurrence-of-a-recurring-event found, non-recurring passthrough unchanged, no double-counting a master that also overlaps the window, `UNTIL` bound honored, unparseable `rrule` degrades to the raw master. Plus an RPC-level test `list_events_excludes_events_from_a_different_calendar_id` for the calendar_id filter. ## Gate evidence ``` cargo fmt --all -- --check -> clean cargo check-all -> clean (clippy --all-targets --workspace -D warnings, 0 warnings) cargo test-all -> 0 failed, 0 ignored across the workspace nuncio-cal: 25 passed nuncio-store: 51 passed nunciod: 91 passed (unit) + calendar_e2e_test 1 passed + daemon_e2e_test 1 + grpc_auth_test 3 + spine_e2e_test 1 (all other workspace crates unaffected, all green) ``` Pre-commit hook (fmt/clippy/test-all) also ran and passed on the commit. ## Decisions / notes for PO review - No proto change — `ListEventsResponse.events` already carries the expanded occurrences; not a `[PROTO]` story. - `nuncio-store` gained no dependency on `nuncio-cal`; expansion happens entirely at the `nunciod` gRPC layer, per the issue's layering guidance. - The `RecurrenceEngine::expand_occurrences` behavior change (removing the "return raw master on empty match" fallback, switching to `skip_while`/`take_while`) is a real engine-level fix, not just RPC wiring — flagging it explicitly since the issue said "no change expected" unless a genuine gap was found. Happy to discuss if a different tradeoff is preferred.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: KofTwentyTwo/nuncio#215