feat(recurrence): interval-based rules (每 N 分鐘/小時/天/週) - #28
Conversation
Extend the RecurrenceRule dataclass with a new INTERVAL period and an interval_seconds field so users can say "每 15 分鐘提醒我喝水" or "每 2 週提醒我倒垃圾". INTERVAL rules do not carry a time-of-day; next_fire just adds the interval to the current tick. Parser gains RECURRENCE_INTERVAL_RE with unit aliases (天/日, 週/周). Below INTERVAL_MIN_SECONDS (10 min) the parser emits a RecurrenceError → router single-turn reject (『每 1 分鐘』太頻繁, 最低支援 10 分鐘). Above the threshold a valid rule is built and title extraction reuses the existing "strip marker + 提醒(我/我們/大家)?" flow; a missing title still falls through to the normal draft prompt. Storage does not need a migration — recurrence is a JSON blob and serialize/deserialize gained an INTERVAL branch that omits hour/minute. format_rule picks the largest evenly-divisible unit so 900s renders as "每 15 分鐘" and 604800s as "每 1 週". render_recurrence_error template dropped the date-specific "不是有效的日期" wrapper; parser reasons are now self-contained (「日期無效，需在 1-31 範圍」/ 「太頻繁，最低支援 10 分鐘」) so both date and interval errors read naturally. Test count: 203 → 216. Generated with AI Co-Authored-By: AI <ai@example.com>
Recurring reminders will fire again on schedule, so the snooze buttons
(10 分/1 小時/明天 HH:MM) inherited from one-off delivery add no signal
over 跳過下次: they either overlap ("明天 09:15" on a DAILY 09:00 rule is
a no-op) or drift the cadence (INTERVAL every 15 min + snooze 10 min ends
up on a shifted grid). Trim delivery_recurring_keyboard down to the two
actions that are actually distinct: skip the next occurrence, or cancel
the whole series. Users who need a precise one-shot delay can /remind a
new reminder.
Signature dropped next_day_time_label since the label was only used by
the snooze buttons. Scheduler now branches so it only computes the label
for one-off reminders.
Generated with AI
Co-Authored-By: AI <ai@example.com>
There was a problem hiding this comment.
Pull request overview
This PR extends Remindly’s recurrence system with an INTERVAL-based recurrence rule (e.g., 每 N 分鐘/小時/天/週), integrating it end-to-end across parsing, serialization, formatting, scheduling, and service flows while keeping DB storage unchanged (JSON blob).
Changes:
- Add
RecurrencePeriod.INTERVAL+RecurrenceRule.interval_seconds, withnext_fire()andformat_rule()support. - Extend the parser to recognize “每 N …” (including 日/天 and 週/周 aliases) and enforce a minimum interval of 10 minutes.
- Update delivery UI behavior so recurring reminders only show “跳過下次 / 取消整個系列” actions (no snooze buttons), with updated tests.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/remindly/reminders/models.py |
Adds INTERVAL period and interval_seconds to the recurrence dataclass model. |
src/remindly/reminders/parser.py |
Parses interval markers and produces interval-based recurrence rules + initial remind_at. |
src/remindly/reminders/recurrence.py |
Adds interval serialization/deserialization, interval next_fire, and interval formatting logic/constants. |
src/remindly/reminders/renderer.py |
Updates recurrence error rendering and simplifies recurring delivery keyboard to 2 actions. |
src/remindly/reminders/scheduler.py |
Uses the simplified recurring keyboard and avoids computing snooze labels for recurring reminders. |
tests/test_parser.py |
Adds coverage for interval parsing, unit aliases, minimum boundary behavior, and title extraction. |
tests/test_recurrence.py |
Adds interval coverage for serialize/deserialize, next_fire, and formatting logic. |
tests/test_service.py |
Adds end-to-end service coverage for interval recurrence creation + minimum interval rejection. |
tests/test_scheduler.py |
Updates expectations for recurring reminder keyboards (no snooze buttons). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # 每 N 分鐘 / 每 N 小時 / 每 N 天 / 每 N 週 — interval | ||
| # 「日/週」是常見別名；n 沒上限（policy 由 INTERVAL_MIN_SECONDS 控），但零和負數 | ||
| # 由 regex 直接排除（\d+ 且後續 unit-multiplier 保證 > 0）。 | ||
| RECURRENCE_INTERVAL_RE = re.compile( | ||
| r"每\s*(?P<n>\d+)\s*(?P<unit>分鐘|小時|天|日|週|周)" | ||
| ) |
There was a problem hiding this comment.
Fixed in 9df43ad. Kept \d+ but split the check: seconds<=0 → reason「間隔需為正整數」, seconds<INTERVAL_MIN_SECONDS → reason「太頻繁」. Preferred this over [1-9]\d* because falling through to the one-off parser and asking「什麼時候提醒？」on 每 0 分鐘 is worse UX than a specific rejection.
| if period == RecurrencePeriod.INTERVAL: | ||
| rule = RecurrenceRule(period=period, **extras) | ||
| remind_at = next_fire(rule, now) | ||
| remaining = cleaned.replace(marker_text, " ", 1) |
There was a problem hiding this comment.
Fixed in 9df43ad. Wrapped next_fire in _try_recurrence_parse with try/except OverflowError and return RecurrenceError(reason="間隔太大，超出可支援範圍"). Test: test_interval_overflow_rejected uses 每 99999999999 週 to trigger the overflow.
| if rule.period == RecurrencePeriod.INTERVAL: | ||
| payload["interval_seconds"] = rule.interval_seconds | ||
| return json.dumps(payload, ensure_ascii=False, sort_keys=True) |
There was a problem hiding this comment.
Fixed in 9df43ad. serialize_rule now validates both cases early: INTERVAL requires positive interval_seconds, non-INTERVAL requires hour/minute. Aligned with the checks already in next_fire and format_rule so bad data cannot reach the DB. Tests: test_serialize_rejects_missing_interval_seconds, test_serialize_rejects_missing_hour_minute.
| if rule.period == RecurrencePeriod.DAILY: | ||
| candidate = after.replace(hour=rule.hour, minute=rule.minute, second=0, microsecond=0) |
There was a problem hiding this comment.
Fixed in 9df43ad. Added an explicit hour/minute is None guard right after the INTERVAL branch in next_fire, mirroring the same pattern in format_rule and serialize_rule. Callers now get a clear ValueError with the field names instead of TypeError: unsupported type for replace(): NoneType. Test: test_non_interval_missing_hour_raises_in_next_fire.
Copilot round 1 flagged four gaps in the INTERVAL feature: 1. RECURRENCE_INTERVAL_RE matches n=0, which falls into the "太頻繁" bucket and emits a misleading reason. Split the check: seconds<=0 → "間隔需為 正整數"; below INTERVAL_MIN_SECONDS → "太頻繁". Kept \d+ so 「每 0 分鐘」 still hits a marker path (a stricter [1-9]\d* would silently fall through to "什麼時候提醒？", worse UX). 2. Very large n (「每 99999999999 週」) raises OverflowError inside `now + timedelta`, crashing begin_create. Catch in the INTERVAL branch of `_try_recurrence_parse` and return RecurrenceError. 3. `serialize_rule` accepted INTERVAL rules without validating interval_seconds, letting bad data reach the DB and blow up deserialize with `int(None)`. Align validation with next_fire / format_rule and raise early. Same guard for missing hour/minute on non-interval rules. 4. Optional hour/minute made `next_fire(RecurrenceRule(period=DAILY))` raise TypeError from `after.replace(hour=None)`. Add explicit None guard on the non-INTERVAL path so callers see ValueError with a readable message. Test count: 216 → 221. Generated with AI Co-Authored-By: AI <ai@example.com>
| # 非 60 倍數不應該由 parser 產生；保底退回分鐘表示 | ||
| return f"每 {seconds // 60} 分鐘" |
There was a problem hiding this comment.
Fixed in 94220aa. Replaced the truncating fallback with a ValueError. Parser guarantees 60-multiples so the normal path still hits _INTERVAL_UNITS; this only fires on out-of-band values (e.g., a DB hand edit produces 601s), which is exactly when a loud failure beats silent drift. Test: test_format_rule_interval_rejects_non_divisible_seconds. 222 tests still green.
Copilot round 2 flagged that `_format_interval_seconds` silently truncates to minutes when interval_seconds does not divide evenly into any supported unit (601s → 「每 10 分鐘」 while the schedule is actually 601s). Parser guarantees a 60-multiple so this is unreachable via the normal path, but if a bad value ever reaches format_rule (e.g. DB hand edit), raising ValueError is more debuggable than a silently drifting UI. Aligns with the defensive raise pattern already used in next_fire / serialize_rule. Generated with AI Co-Authored-By: AI <ai@example.com>
| # 2028 是閏年：2 月最多 29 天（跟 `is_valid_month_day` 一致） | ||
| max_day = calendar.monthrange(2028, month)[1] |
There was a problem hiding this comment.
Fixed in cd5e4bc. Prepended 「日期無效」 to the yearly impossible-date reason so it now renders as 「『每年 2/30』日期無效，2 月最多 29 天。」, matching monthly (「日期無效，需在 1-31 範圍」) and interval (「太頻繁，最低支援 10 分鐘」) tone. Updated test_yearly_rejects_impossible_date + test_yearly_rejects_april_31 accordingly.
Copilot round 3 noted that yearly "day exceeds month max" produced a
reason like "2 月最多 29 天" — reads as a statement of fact instead of a
rejection, unlike monthly ("日期無效，需在 1-31 範圍") and interval
("太頻繁，最低支援 10 分鐘") which carry their own reject-tone prefix.
Prepend "日期無效" so render_recurrence_error emits a consistent
message: 「『每年 2/30』日期無效，2 月最多 29 天。」.
Generated with AI
Co-Authored-By: AI <ai@example.com>
Summary
RecurrenceRule加入INTERVALperiod 與interval_seconds欄位（B 方案：延續現有 dataclass 架構，避免引入python-dateutil依賴）。RECURRENCE_INTERVAL_RE，接受單位別名（天/日、週/周）；低於 10 分鐘走單輪拒絕（『每 1 分鐘』太頻繁，最低支援 10 分鐘）。recurrence欄位是 JSON blob，serialize/deserialize加 INTERVAL 分支即可，不需 DB migration。format_rule挑最大能整除的單位輸出（900s → 每 15 分鐘、604800s → 每 1 週）；next_fireINTERVAL 分支就是after + timedelta(seconds=N)。render_recurrence_error模板拿掉「不是有效的日期」硬字串；parser reason 改成自帶語意，讓 date 與 interval 錯誤都自然。設計取捨（B vs A）
python-dateutil；表達力高但打破 stdlib-only 承諾，且會逼一次 DB migration 把現有 daily/weekly/monthly/yearly 轉字串。Design decisions confirmed with user
每 N 分鐘/每 N 小時/每 N 天/每 N 週，不帶時間每 10 分鐘提醒）走原本 draft 追問，不算單輪拒絕（規則有效、事項缺）Test plan
每 1 分鐘→ RecurrenceError，marker_text="每 1 分鐘"、reason 含「太頻繁」+「10 分鐘」每 10 分鐘剛好等於下限 → 接受每 10 分鐘提醒沒事項 → rule 有效、title 為空、走 draft 追問serialize_ruleINTERVAL 不寫 hour/minute；deserialize_rule反向 round-tripnext_fireINTERVAL = after + timedelta；missing interval_seconds → ValueErrorformat_rule對 600/900/3600/86400/604800 等秒數挑正確單位每 15 分鐘提醒我喝水→ Confirmation with rule + title；每 1 分鐘…→ RecurrenceRejected