#66 — Tech Debt: Monthly Recurrence Permanently Drifts Off Its Original Day-of-Month #66

New issue

Closed

opened 2026-08-18 13:13:26 +02:00 by lena · 1 comment

lena commented

2026-08-18 13:13:26 +02:00

(Migrated from git.butzei.de)

Tech Debt: Monthly Recurrence Permanently Drifts Off Its Original Day-of-Month

Reported by: /code-review, spotted during #20's code review, 2026-07-22

Symptom

RecurrenceCalculator.NextOccurrence (CqsTodo/Features/Todos/RecurrenceCalculator.cs) computes
Monthly recurrence as current.AddMonths(1) applied to the previous occurrence's own due
date, not a separately-stored "anchor day of month" — this is intentional per #20's design (no
extra field; the day-of-month is whatever DueDate already encodes).

DateOnly.AddMonths clamps to the last valid day of a shorter target month. Because each new
occurrence's due date becomes the anchor for computing the next one, a clamp is never undone:

Todo due Jan 31, Monthly. Completed → next due Feb 28 (clamped, expected).
That todo (due Feb 28) completed → next due Mar 28 — not Mar 31 — because
AddMonths(1) is applied to Feb 28, the already-clamped value, not the original 31.
The recurrence day has now permanently drifted from "last day of month" down to a fixed "28th"
for every month going forward, silently changing the user's chosen schedule with no notice.

Daily and Weekly recurrence are unaffected (no clamping occurs for +1 day / +7 days).

Expected behaviour

Monthly recurrence should either:

Preserve the original day-of-month across clamps (e.g. Jan 31 → Feb 28 → Mar 31, not Mar 28) —
requires storing that anchor separately from the current DueDate, since the clamped value
itself no longer carries the information; or
Explicitly document (and ideally surface in the UI) that Monthly recurrence anchors on
whatever day the previous occurrence happened to fall on, so a user picking the 31st
understands their schedule will settle into a shorter day after the first short month.

Acceptance criteria

A todo due Jan 31 with Monthly recurrence, completed on time every month, is due on the 31st
(or the last day of the month, for months with <31 days) every month — not permanently reduced
to the 28th/29th after the first February
A test proves multi-month drift does not occur (Jan 31 → Feb → Mar → ... → Dec, asserting
each due date lands on the expected day)
No change to Daily/Weekly recurrence behaviour

Notes for whoever picks this up

This is the direct trade-off #20's design doc accepted ("no separate day-of-week/day-of-month
field") — fixing it properly likely means reopening that decision (adding an anchor field) rather
than a small patch to RecurrenceCalculator. Low urgency: only affects Monthly recurrence
specifically anchored on the 29th/30th/31st of a month, and only after the first month-length
mismatch.

Agents involved

Backend Engineer — owns evaluating whether to add an anchor field or reword the story's
accepted trade-off

Blockers

None.

# Tech Debt: Monthly Recurrence Permanently Drifts Off Its Original Day-of-Month **Reported by:** `/code-review`, spotted during `#20`'s code review, 2026-07-22 ## Symptom `RecurrenceCalculator.NextOccurrence` (`CqsTodo/Features/Todos/RecurrenceCalculator.cs`) computes `Monthly` recurrence as `current.AddMonths(1)` applied to the **previous occurrence's own due date**, not a separately-stored "anchor day of month" — this is intentional per `#20`'s design (no extra field; the day-of-month is whatever `DueDate` already encodes). `DateOnly.AddMonths` clamps to the last valid day of a shorter target month. Because each new occurrence's due date becomes the anchor for computing the *next* one, a clamp is never undone: 1. Todo due **Jan 31**, Monthly. Completed → next due **Feb 28** (clamped, expected). 2. That todo (due Feb 28) completed → next due **Mar 28** — not **Mar 31** — because `AddMonths(1)` is applied to Feb 28, the already-clamped value, not the original 31. 3. The recurrence day has now permanently drifted from "last day of month" down to a fixed "28th" for every month going forward, silently changing the user's chosen schedule with no notice. Daily and Weekly recurrence are unaffected (no clamping occurs for `+1 day` / `+7 days`). ## Expected behaviour Monthly recurrence should either: - Preserve the *original* day-of-month across clamps (e.g. Jan 31 → Feb 28 → Mar 31, not Mar 28) — requires storing that anchor separately from the current `DueDate`, since the clamped value itself no longer carries the information; or - Explicitly document (and ideally surface in the UI) that Monthly recurrence anchors on whatever day the *previous* occurrence happened to fall on, so a user picking the 31st understands their schedule will settle into a shorter day after the first short month. ## Acceptance criteria - [ ] A todo due Jan 31 with Monthly recurrence, completed on time every month, is due on the 31st (or the last day of the month, for months with <31 days) every month — not permanently reduced to the 28th/29th after the first February - [ ] A test proves multi-month drift does not occur (Jan 31 → Feb → Mar → ... → Dec, asserting each due date lands on the expected day) - [ ] No change to Daily/Weekly recurrence behaviour ## Notes for whoever picks this up This is the direct trade-off `#20`'s design doc accepted ("no separate day-of-week/day-of-month field") — fixing it properly likely means reopening that decision (adding an anchor field) rather than a small patch to `RecurrenceCalculator`. Low urgency: only affects Monthly recurrence specifically anchored on the 29th/30th/31st of a month, and only after the first month-length mismatch. ## Agents involved - **Backend Engineer** — owns evaluating whether to add an anchor field or reword the story's accepted trade-off ## Blockers None.

lena commented

2026-08-18 13:13:26 +02:00

(Migrated from git.butzei.de)

design (66_recurring_todo_monthly_day_drift_design.md)

`#66` — Design Note

Decision: reopen `#20`'s design, add the anchor field

Per the story's own framing, this required revisiting #20's accepted trade-off ("no separate
day-of-week/day-of-month field"). Added TodoEntity.RecurrenceAnchorDay (nullable int, plain —
not a Vogen VO, matching SortOrder's precedent for an internal, non-user-facing integer) rather
than reworking the whole recurrence model — Weekly still needs no anchor at all (no clamping
occurs), only Monthly does.

Mechanics

RecurrenceCalculator.NextOccurrence gains a required anchorDay parameter. For Monthly:
current.AddMonths(1) is still used, but only for its year/month — its own day is discarded
(that's the value the whole bug was about trusting) and replaced with
Math.Min(anchorDay, DateTime.DaysInMonth(targetYear, targetMonth)), computed fresh against the
fixed anchor every call. Jan 31 (anchor 31) → Feb 28 → Mar 31 → Apr 30, never drifting to a
permanently-reduced day the way current.AddMonths(1) chained on itself would.
TodoEntity.RecurrenceAnchorDay is set in exactly one of two ways, both in
SetTodoRecurrenceCommandHandler: derived from the todo's own current due date's day when
recurrence is set for the first time (the natural, only-sensible anchor at that moment), or
passed verbatim via a new AnchorDayOverride constructor parameter when CheckTodoCommandHandler
carries an existing recurrence forward onto a freshly-spawned next occurrence — carrying
forward must reuse the original anchor unchanged, never recompute one from the new occurrence's
own due date (which may already be a clamped value). Same "public command, trusted
caller-supplied override" shape as the existing AssignTodoCommand.NotifyAssignee precedent.
RollForwardMissedRecurringTodosCommandHandler's multi-step catch-up loop (a todo missed
several months while the app was down) also threads the same fixed anchor through every
iteration, for the same reason.
Clearing recurrence (RecurrenceRule: null) clears the anchor too, keeping the two fields in
lockstep — a todo with no recurrence rule never has a stale anchor value sitting around.

Existing recurring todos (pre-migration data)

Rows that already had RecurrenceRule set before this migration get RecurrenceAnchorDay = null
(the new column's default) — their day-of-month may already be drifted from whatever the user
originally intended, and this migration doesn't attempt to reconstruct history to guess the
original anchor. Every read path that consumes the anchor falls back to ?? dueDate.Day when
null, so behavior for already-drifted todos is unchanged (no regression) until the next
transition (completion or missed-rollover), at which point a real anchor gets established from
whatever due date is current at that moment and drift stops going forward from there. Not
retroactively fixed, but self-healing on the next natural transition rather than requiring a
backfill migration.

Test coverage

RecurrenceCalculatorTests.cs: 2 new tests directly reproduce the ticket's own bug scenario
(Jan 31 → Feb 28 → Mar 31 → Apr 30, and a 30th-anchored case that clamps every February but
recovers every other month) — both pass locally (pure C#, no DB needed).
SetTodoRecurrenceCommandHandlerTests.cs: 3 new tests cover first-time anchor derivation,
AnchorDayOverride taking precedence, and anchor clearing alongside rule clearing.
RollForwardMissedRecurringTodosCommandHandlerTests.cs gained a multi-month catch-up test
(a todo anchored on the 31st, missed for over a year, rolled forward in one sweep) — code review
correctly flagged the initial omission of this case as a real gap, since it's the literal
multi-month scenario the ticket describes. The test derives its expected end date by calling the
same RecurrenceCalculator.NextOccurrence the handler itself uses, in a loop, rather than
hand-computing a long calendar chain — an integration-level check that the handler correctly
threads the persisted anchor through every catch-up iteration, layered on top of
RecurrenceCalculatorTests.cs's already-thorough direct coverage of the calculation itself.

**design** (`66_recurring_todo_monthly_day_drift_design.md`) # `#66` — Design Note ## Decision: reopen `#20`'s design, add the anchor field Per the story's own framing, this required revisiting `#20`'s accepted trade-off ("no separate day-of-week/day-of-month field"). Added `TodoEntity.RecurrenceAnchorDay` (nullable `int`, plain — not a Vogen VO, matching `SortOrder`'s precedent for an internal, non-user-facing integer) rather than reworking the whole recurrence model — Weekly still needs no anchor at all (no clamping occurs), only Monthly does. ## Mechanics - `RecurrenceCalculator.NextOccurrence` gains a required `anchorDay` parameter. For Monthly: `current.AddMonths(1)` is still used, but *only* for its year/month — its own day is discarded (that's the value the whole bug was about trusting) and replaced with `Math.Min(anchorDay, DateTime.DaysInMonth(targetYear, targetMonth))`, computed fresh against the fixed anchor every call. Jan 31 (anchor 31) → Feb 28 → Mar 31 → Apr 30, never drifting to a permanently-reduced day the way `current.AddMonths(1)` chained on itself would. - `TodoEntity.RecurrenceAnchorDay` is set in exactly one of two ways, both in `SetTodoRecurrenceCommandHandler`: derived from the todo's own current due date's day when recurrence is set for the first time (the natural, only-sensible anchor at that moment), or passed verbatim via a new `AnchorDayOverride` constructor parameter when `CheckTodoCommandHandler` carries an *existing* recurrence forward onto a freshly-spawned next occurrence — carrying forward must reuse the original anchor unchanged, never recompute one from the new occurrence's own due date (which may already be a clamped value). Same "public command, trusted caller-supplied override" shape as the existing `AssignTodoCommand.NotifyAssignee` precedent. - `RollForwardMissedRecurringTodosCommandHandler`'s multi-step catch-up loop (a todo missed several months while the app was down) also threads the same fixed anchor through every iteration, for the same reason. - Clearing recurrence (`RecurrenceRule: null`) clears the anchor too, keeping the two fields in lockstep — a todo with no recurrence rule never has a stale anchor value sitting around. ## Existing recurring todos (pre-migration data) Rows that already had `RecurrenceRule` set before this migration get `RecurrenceAnchorDay = null` (the new column's default) — their day-of-month may already be drifted from whatever the user originally intended, and this migration doesn't attempt to reconstruct history to guess the original anchor. Every read path that consumes the anchor falls back to `?? dueDate.Day` when null, so behavior for already-drifted todos is unchanged (no regression) until the *next* transition (completion or missed-rollover), at which point a real anchor gets established from whatever due date is current at that moment and drift stops going forward from there. Not retroactively fixed, but self-healing on the next natural transition rather than requiring a backfill migration. ## Test coverage `RecurrenceCalculatorTests.cs`: 2 new tests directly reproduce the ticket's own bug scenario (Jan 31 → Feb 28 → Mar 31 → Apr 30, and a 30th-anchored case that clamps every February but recovers every other month) — both pass locally (pure C#, no DB needed). `SetTodoRecurrenceCommandHandlerTests.cs`: 3 new tests cover first-time anchor derivation, `AnchorDayOverride` taking precedence, and anchor clearing alongside rule clearing. `RollForwardMissedRecurringTodosCommandHandlerTests.cs` gained a multi-month catch-up test (a todo anchored on the 31st, missed for over a year, rolled forward in one sweep) — code review correctly flagged the initial omission of this case as a real gap, since it's the literal multi-month scenario the ticket describes. The test derives its expected end date by calling the same `RecurrenceCalculator.NextOccurrence` the handler itself uses, in a loop, rather than hand-computing a long calendar chain — an integration-level check that the handler correctly threads the persisted anchor through every catch-up iteration, layered on top of `RecurrenceCalculatorTests.cs`'s already-thorough direct coverage of the calculation itself.