fix: single execution-date anchor across delete+rebuild (midnight race)

custom_components/nestquest/dao_instances.py

@@ -47,6 +47,18 @@ from .db import NestQuestDatabase
 _UTC_TIMESTAMP_FORMAT = "%Y-%m-%dT%H:%M:%S+00:00"
 
 
+def _today() -> datetime.date:
+    """Return the current local calendar date (the single date source).
+
+    Every "today" read in this module goes through this one function so
+    the delete clamp and the materialize walk's no-past guard read the
+    same clock, and a caller or test can pin/roll that clock
+    deterministically (e.g. a queued-across-midnight regression).  Pure
+    clock read — no business rule lives here.
+    """
+    return datetime.date.today()
+
+
 def _validate_utc_timestamp(value: str, field: str) -> None:
     """Raise ValueError unless ``value`` is a strict UTC ISO-8601 stamp.
 
@@ -203,7 +215,7 @@ class QuestInstancesDao:
         # check runs after the lock is acquired, because a caller can
         # wait on the lock across midnight — a date that was "today"
         # at call time may be "yesterday" by the time the INSERT runs.
-        today = datetime.date.today().isoformat()
+        today = _today().isoformat()
         if due_date < today:
             raise ValueError(
                 f"instances are never generated in the past: due_date "
@@ -214,7 +226,7 @@ class QuestInstancesDao:
                 # Re-read "today" under the lock: this is the date the
                 # insert actually executes on, so the no-past rule
                 # holds across midnight rollovers.
-                today = datetime.date.today().isoformat()
+                today = _today().isoformat()
                 if due_date < today:
                     raise ValueError(
                         f"instances are never generated in the past: "
@@ -326,7 +338,7 @@ class QuestInstancesDao:
         _validate_window(window)
         # Fail fast on obviously-past dates before queuing on the lock;
         # the authoritative check re-runs under the lock (see upsert).
-        today = datetime.date.today().isoformat()
+        today = _today().isoformat()
         if due_date < today:
             raise ValueError(
                 f"instances are never generated in the past: due_date "
@@ -334,7 +346,7 @@ class QuestInstancesDao:
             )
         async with _connection_lock(self._database):
             async with self._database.transaction():
-                today = datetime.date.today().isoformat()
+                today = _today().isoformat()
                 if due_date < today:
                     raise ValueError(
                         f"instances are never generated in the past: "
@@ -475,7 +487,7 @@ class QuestInstancesDao:
                 # midnight cannot delete instances that became past
                 # while it waited (backdating is clamped to the
                 # execution date, not the call date).
-                today = datetime.date.today().isoformat()
+                today = _today().isoformat()
                 effective_cutoff = max(cutoff_date, today)
                 result = await self._database.execute(
                     "DELETE FROM quest_instances WHERE definition_id = ? "
@@ -489,9 +501,9 @@ class QuestInstancesDao:
 
     async def delete_future_uncompleted_for_child(
         self, child_id: int, cutoff_date: str
-    ) -> int:
+    ) -> tuple[int, str]:
         """Delete the child's open instances at/after cutoff, across every
-        definition.
+        definition, returning ``(deleted, effective_cutoff)``.
 
         Child-scoped counterpart to :meth:`delete_future_uncompleted`,
         used when a child's presence changes (Feature 09): only instances
@@ -500,7 +512,16 @@ class QuestInstancesDao:
         mid-flight cannot be deleted.  The cutoff cannot be backdated below
         today, for the same reason as the definition-scoped method (see its
         docstring).  Any instance with a completion event is never touched,
-        regardless of its date.  Returns the number deleted.
+        regardless of its date.
+
+        Returns the number deleted and the effective cutoff that was
+        actually applied (``max(cutoff_date, today)``, read under the
+        lock).  The caller (:func:`~.materialize.regenerate_for_child`)
+        passes that effective cutoff back as its re-materialization anchor,
+        so the delete and the rebuild share ONE execution-day even when the
+        call is queued across midnight — instead of the caller re-reading a
+        stale "today" and materializing a window the delete already
+        superseded.
         """
         _validate_date(cutoff_date, "cutoff_date")
         async with _connection_lock(self._database):
@@ -509,7 +530,7 @@ class QuestInstancesDao:
                 # definition-scoped method: the DELETE executes on this
                 # date, so a caller queued across midnight cannot delete
                 # instances that became past while it waited.
-                today = datetime.date.today().isoformat()
+                today = _today().isoformat()
                 effective_cutoff = max(cutoff_date, today)
                 result = await self._database.execute(
                     "DELETE FROM quest_instances WHERE child_id = ? "
@@ -519,7 +540,7 @@ class QuestInstancesDao:
                     ")",
                     (child_id, effective_cutoff),
                 )
-        return result.rowcount
+        return result.rowcount, effective_cutoff
 
 
 class CompletionEventsDao:

custom_components/nestquest/materialize.py

@@ -64,7 +64,7 @@ from __future__ import annotations
 import datetime
 
 from .const import DEFAULT_HORIZON_DAYS
-from .dao_instances import QuestInstancesDao
+from .dao_instances import QuestInstancesDao, _today
 from .dao_rules import (
     _validate_date,
     load_materialization_input,
@@ -207,14 +207,14 @@ async def regenerate_for_definition(
     assignee set and windows re-materialize against today's state.
 
     "today" is computed the same way the walk computes it —
-    ``datetime.date.today()`` — so the delete cutoff and the
+    :func:`~.dao_instances._today` — so the delete cutoff and the
     re-materialization range share one anchor, and the walk's
     ``upsert_if_valid`` no-past guard never rejects the regenerated
     range.  Returns the number of instances the re-materialization
     upserted across the whole snapshot (idempotent — other active
     definitions' tuples refresh in place, never duplicate).
     """
-    today = datetime.date.today()
+    today = _today()
     start_date = today.isoformat()
     end_date = (today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS)).isoformat()
 
@@ -256,16 +256,27 @@ async def regenerate_for_child(
     now-absent/present dates re-materialize against today's presence.
 
     "today" is computed the same way the walk computes it —
-    ``datetime.date.today()`` — so the delete cutoff and the
-    re-materialization range share one anchor.  Returns the number of
-    instances the re-materialization upserted across the whole snapshot
-    (idempotent — other children's and unaffected definitions' tuples
-    refresh in place, never duplicate).
+    :func:`~.dao_instances._today` — and the re-materialization range uses the
+    delete's returned effective cutoff (see
+    :meth:`~.dao_instances.QuestInstancesDao.delete_future_uncompleted_for_child`),
+    so the delete cutoff and the re-materialization window share ONE
+    execution-day anchor even when the call is queued across midnight.
+    Returns the number of instances the re-materialization upserted
+    across the whole snapshot (idempotent — other children's and
+    unaffected definitions' tuples refresh in place, never duplicate).
     """
-    today = datetime.date.today()
-    start_date = today.isoformat()
-    end_date = (today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS)).isoformat()
+    today = _today()
 
     instances = QuestInstancesDao(database)
-    await instances.delete_future_uncompleted_for_child(child_id, start_date)
-    return await materialize(database, start_date, end_date)
+    # Use the delete's returned effective cutoff as the materialize anchor,
+    # NOT the independently-computed "today" above.  If the call is queued
+    # across midnight the delete clamps its cutoff to the execution-day;
+    # materializing from the stale "today" (now yesterday) would trip the
+    # walk's no-past guard and leave the child's future instances deleted
+    # but not rebuilt.  One anchor for both keeps them consistent.
+    _, effective_cutoff = await instances.delete_future_uncompleted_for_child(
+        child_id, today.isoformat()
+    )
+    start = datetime.date.fromisoformat(effective_cutoff)
+    end_date = (start + datetime.timedelta(days=DEFAULT_HORIZON_DAYS)).isoformat()
+    return await materialize(database, start.isoformat(), end_date)

tests/test_materialize.py

@@ -886,4 +886,79 @@ def test_regenerate_for_child_rebuilds_horizon_preserves_history(
         assert past_due in by_due
         return None
 
-    _with_db(tmp_path, "regenerate-child.db")(_body)
+    _with_db(tmp_path, "regenerate-child.db")(_body)
+
+
+def test_regenerate_for_child_uses_single_anchor_across_midnight(
+    tmp_path, monkeypatch
+) -> None:
+    """A call queued across midnight must not delete-then-fail-to-rebuild.
+
+    ``regenerate_for_child`` reads "today" once as a cutoff hint, but the
+    delete clamps that cutoff to the execution-day under its own lock and
+    RETURNS the effective cutoff; the re-materialization window is then
+    anchored to the same returned day.  This test fakes a midnight rollover
+    between the two reads: the caller's hint is day0 while the delete
+    executes on day1.  The rebuild must start on day1 (so the walk's
+    no-past guard never fires) and the now-past day0 instance must survive.
+    """
+    day0 = datetime.date.today() + datetime.timedelta(days=30)
+    day1 = day0 + datetime.timedelta(days=1)
+
+    clock = {"rolled": False, "reads": 0}
+
+    def _fake_today() -> datetime.date:
+        if not clock["rolled"]:
+            return day0
+        clock["reads"] += 1
+        return day0 if clock["reads"] == 1 else day1
+
+    import custom_components.nestquest.dao_instances as dao_instances_module
+    import custom_components.nestquest.materialize as materialize_module
+
+    monkeypatch.setattr(dao_instances_module, "_today", _fake_today)
+    monkeypatch.setattr(materialize_module, "_today", _fake_today)
+
+    async def _body(database):
+        children = ChildrenDao(database)
+        child = await children.create("Ada", NOW)
+
+        day0_iso = day0.isoformat()
+        horizon_end = day0 + datetime.timedelta(days=14)
+
+        created = await create_quest_definition(
+            database,
+            "Daily chore",
+            ScheduleRule(rule_type=RuleType.DAILY, start_date=day0_iso),
+            [child.id],
+            [("morning", "09:00")],
+        )
+        dao = QuestInstancesDao(database)
+
+        # Seed the horizon while the clock reads day0.
+        await materialize(database, day0_iso, horizon_end.isoformat())
+
+        # Roll the clock forward: every read after the caller's single
+        # cutoff hint now returns day1 (the delete's execution-day).
+        clock["rolled"] = True
+        clock["reads"] = 0
+
+        rebuilt = await regenerate_for_child(database, child.id)
+
+        # No raise occurred; the rebuilt window is anchored on the delete's
+        # execution-day (day1), so day1..day1+14 all exist.
+        assert rebuilt == 15
+        day1_iso = day1.isoformat()
+        day1_horizon_end = day1 + datetime.timedelta(days=14)
+        assert len(await dao.list_by_date_range(
+            child.id, day1_iso, day1_horizon_end.isoformat()
+        )) == 15
+
+        # day0 was the caller's "today" but rolled over to the past by the
+        # time the delete ran; it must survive (never delete the past).
+        assert await dao.get(
+            created.definition.id, child.id, day0_iso, "morning"
+        ) is not None
+        return rebuilt
+
+    _with_db(tmp_path, "regenerate-child-midnight.db")(_body)