From fac9f483ef39fababcfc3a02505b512600ba44e6 Mon Sep 17 00:00:00 2001 From: Joshua Date: Thu, 17 Sep 2026 13:59:59 -0400 Subject: [PATCH] fix: store the snapshot due_time, not the live value (NQ-75-CR-002) --- custom_components/nestquest/dao_instances.py | 19 +++++---- custom_components/nestquest/materialize.py | 1 + tests/test_materialize.py | 42 +++++++++++++++++++- 3 files changed, 53 insertions(+), 9 deletions(-) diff --git a/custom_components/nestquest/dao_instances.py b/custom_components/nestquest/dao_instances.py index 36d7000..841c1a7 100644 --- a/custom_components/nestquest/dao_instances.py +++ b/custom_components/nestquest/dao_instances.py @@ -285,6 +285,7 @@ class QuestInstancesDao: generated_at: str, *, window: str, + due_time: str | None = None, ) -> QuestInstanceRecord | None: """Insert one dated instance iff its tuple is still materializable. @@ -301,9 +302,14 @@ class QuestInstancesDao: - the (definition, child) assignment link still exists (a removed assignment is skipped); - the window is still declared on the definition (a removed window - is skipped), and its CURRENT ``due_time`` is read from the live - row so a due-time edit landing after the snapshot is honored, - never the stale snapshot value. + is skipped). + + The live checks above ONLY decide whether the tuple is still + valid; they never read the window's ``due_time``. The instance is + written with the caller-supplied ``due_time`` — the value snapshotted + from the coherent input read — so a window-time edit landing + mid-walk cannot leak a newer time into a batch that was already + snapshotted against older rule/presence/assignment decisions. Returns the stored instance, or ``None`` when a precondition failed — the tuple no longer materializable, which the walk SKIPS @@ -330,8 +336,8 @@ class QuestInstancesDao: f"instances are never generated in the past: " f"due_date {due_date!r} is before today {today!r}" ) - valid = await self._database.fetch_one( - "SELECT w.due_time FROM quest_definitions d " + still_valid = await self._database.fetch_one( + "SELECT 1 FROM quest_definitions d " "JOIN quest_definition_assignees a " "ON a.definition_id = d.id " "JOIN children c ON c.id = a.child_id " @@ -341,11 +347,10 @@ class QuestInstancesDao: "AND d.is_active = 1 AND c.is_active = 1 LIMIT 1", (definition_id, child_id, window), ) - if valid is None: + if still_valid is None: # A config change invalidated the tuple since the # snapshot: skip it rather than abort the batch. return None - due_time = valid[0] existing = await self._database.fetch_one( "SELECT 1 FROM completion_events " "WHERE instance_id = (SELECT id FROM quest_instances " diff --git a/custom_components/nestquest/materialize.py b/custom_components/nestquest/materialize.py index 387380a..5c4de5b 100644 --- a/custom_components/nestquest/materialize.py +++ b/custom_components/nestquest/materialize.py @@ -159,6 +159,7 @@ async def materialize( iso, generated_at, window=window.window, + due_time=window.due_time, ) if written is not None: count += 1 diff --git a/tests/test_materialize.py b/tests/test_materialize.py index ff9dace..86dfe31 100644 --- a/tests/test_materialize.py +++ b/tests/test_materialize.py @@ -248,7 +248,7 @@ async def _materialize_with_change( seen = {"n": 0} async def _gated(self, definition_id, child_id, due_date, generated_at, - *, window): + *, window, due_time=None): seen["n"] += 1 if seen["n"] == 1: seen["first"] = (definition_id, child_id, window) @@ -256,7 +256,7 @@ async def _materialize_with_change( await release.wait() return await original_upsert( self, definition_id, child_id, due_date, generated_at, - window=window, + window=window, due_time=due_time, ) QuestInstancesDao.upsert_if_valid = _gated @@ -452,6 +452,44 @@ def test_materialize_uses_single_presence_snapshot_mid_walk(tmp_path) -> None: _with_db(tmp_path, "materialize-presence-snapshot.db")(_body) +def test_materialize_stores_snapshot_due_time_not_live(tmp_path) -> None: + async def _body(database): + children = ChildrenDao(database) + child = await children.create("Ada", NOW) + start = _future_monday() + start_iso = start.isoformat() + end_iso = (start + datetime.timedelta(days=2)).isoformat() + created = await create_quest_definition( + database, + "Daily chore", + ScheduleRule(rule_type=RuleType.DAILY, start_date=start_iso), + [child.id], + [("morning", "09:00")], + ) + + async def _edit_due_time(): + await QuestDefinitionsDao(database).upsert_window( + created.definition.id, "morning", due_time="10:15" + ) + + # The window's due_time is edited to 10:15 between the input + # snapshot and the insert. The walk must store the SNAPSHOT's + # 09:00, not the live 10:15 — the batch is governed entirely by + # the coherent snapshot it read. + count = await _materialize_with_change( + database, start_iso, end_iso, _edit_due_time + ) + assert count == 3 + records = await QuestInstancesDao(database).list_by_date_range( + child.id, start_iso, end_iso + ) + assert len(records) == 3 + assert {r.due_time for r in records} == {"09:00"} + return records + + _with_db(tmp_path, "materialize-snapshot-due-time.db")(_body) + + def test_materialize_honours_overrides(tmp_path) -> None: async def _body(database): from custom_components.nestquest.dao_presence import (