diff --git a/custom_components/nestquest/dao_instances.py b/custom_components/nestquest/dao_instances.py index 5079f27..c948aa8 100644 --- a/custom_components/nestquest/dao_instances.py +++ b/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: diff --git a/custom_components/nestquest/materialize.py b/custom_components/nestquest/materialize.py index 5af3626..271d0de 100644 --- a/custom_components/nestquest/materialize.py +++ b/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) diff --git a/tests/test_materialize.py b/tests/test_materialize.py index 3e50ed0..17caad6 100644 --- a/tests/test_materialize.py +++ b/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) \ No newline at end of file + _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) \ No newline at end of file