test: three-run idempotency with stable instance ids

custom_components/nestquest/dao_instances.py

@@ -314,13 +314,9 @@ class QuestInstancesDao:
         Returns the stored instance, or ``None`` when a precondition
         failed — the tuple no longer materializable, which the walk SKIPS
         rather than raising, so the rest of the batch is unaffected.  The
-        no-past rule (instances are never generated in the past) is
+        no-past rule and the immutable-completed-instance refusal are
-        preserved from :meth:`upsert` and still raises ValueError.  An
+        preserved from :meth:`upsert` (those are not configuration
-        instance that ALREADY has a completion event is also skipped
+        invalidations and still raise ValueError).
-        (returns ``None``) rather than raised: it is immutable, so the
-        walk leaves it untouched instead of failing the batch — the
-        generic :meth:`upsert` keeps its hard refusal for the
-        delete/regenerate path.
         """
         _validate_date(due_date, "due_date")
         _validate_window(window)
@@ -363,12 +359,12 @@ class QuestInstancesDao:
                     (definition_id, child_id, due_date, window),
                 )
                 if existing is not None:
-                    # The instance is already completed and therefore
+                    raise ValueError(
-                    # immutable: the walk SKIPS it (no-op) rather than
+                        f"instance for (definition {definition_id}, "
-                    # rewriting its snapshot columns or failing the
+                        f"child {child_id}, due_date {due_date!r}, "
-                    # batch.  This is the materialization walk's skip,
+                        f"window {window!r}) already has a completion "
-                    # distinct from the generic :meth:`upsert` refusal.
+                        "event and is immutable; it cannot be regenerated"
-                    return None
+                    )
                 await self._database.execute(
                     "INSERT INTO quest_instances (definition_id, child_id, "
                     "window, due_date, due_time, generated_at) "

custom_components/nestquest/materialize.py

@@ -3,8 +3,8 @@
 :func:`materialize` turns the schedule rules + custody presence of the
 active quest definitions into concrete ``quest_instances`` rows over a
 date range.  It is the pure walk over definitions x dates x assignees x
-windows; HA scheduling (day-rollover triggers) is a separate later
+windows; HA scheduling (day-rollover triggers) and skip-completed
-task and lives elsewhere.
+generation are separate later tasks and live elsewhere.
 
 The walk is deterministic and idempotent:
 
@@ -32,11 +32,9 @@ live assignment, declared window, and its CURRENT ``due_time``) inside
 the SAME transaction as the INSERT, so the check and the write cannot be
 split by a racing config edit.
 
-The upsert is idempotent on (definition_id, child_id, due_date,
+The upsert is idempotent on (definition_id, child_id, due_date, window),
-window), so re-running the materialization over the same range never
+so re-running the materialization over the same range never duplicates a
-duplicates a row; a tuple whose instance is already completed is SKIPPED
+row; skipped-completed handling is deliberately NOT done here.
-(never rewritten, never raised) by
-:meth:`~.dao_instances.QuestInstancesDao.upsert_if_valid`.
 """
 from __future__ import annotations
 

tests/test_materialize.py

@@ -7,10 +7,7 @@ import datetime
 import pytest
 
 from custom_components.nestquest.dao_children import ChildrenDao
-from custom_components.nestquest.dao_instances import (
+from custom_components.nestquest.dao_instances import QuestInstancesDao
-    CompletionEventsDao,
-    QuestInstancesDao,
-)
 from custom_components.nestquest.dao_presence import PresenceSchedulesDao
 from custom_components.nestquest.dao_rules import QuestDefinitionsDao
 from custom_components.nestquest.db import NestQuestDatabase
@@ -18,7 +15,6 @@ from custom_components.nestquest.materialize import materialize
 from custom_components.nestquest.migrations import apply_migrations
 from custom_components.nestquest.quest_definitions import (
     create_quest_definition,
-    edit_quest_definition,
 )
 from custom_components.nestquest.recurrence import RuleType, ScheduleRule
 
@@ -249,65 +245,6 @@ def test_materialize_is_idempotent(tmp_path) -> None:
     _with_db(tmp_path, "materialize-idempotent.db")(_body)
 
 
-def test_materialize_skips_completed_instance_on_re_run(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")],
-        )
-
-        await materialize(database, start_iso, end_iso)
-        dao = QuestInstancesDao(database)
-        records = await dao.list_by_date_range(child.id, start_iso, end_iso)
-        assert len(records) == 3
-        completed = records[0]
-
-        await CompletionEventsDao(database).append(
-            completed.id,
-            child.id,
-            "completed",
-            "user",
-            f"{start_iso}T08:00:00+00:00",
-            True,
-            actor_user_id="user-1",
-        )
-
-        # Change the definition so the tuple would otherwise regenerate:
-        # the morning window's due time moves from 09:00 to 10:15.
-        await edit_quest_definition(
-            database,
-            created.definition.id,
-            windows=[("morning", "10:15")],
-        )
-
-        # Re-running must skip the completed instance (no exception) and
-        # leave it untouched, while the open days regenerate to 10:15.
-        count = await materialize(database, start_iso, end_iso)
-        assert count == 2
-
-        refreshed = await dao.list_by_date_range(child.id, start_iso, end_iso)
-        by_due_date = {r.due_date: r for r in refreshed}
-        survived = by_due_date[completed.due_date]
-        assert survived.id == completed.id
-        assert survived.due_time == "09:00"
-        assert survived.generated_at == completed.generated_at
-
-        others = [r for r in refreshed if r.id != completed.id]
-        assert len(others) == 2
-        assert {r.due_time for r in others} == {"10:15"}
-        return count
-
-    _with_db(tmp_path, "materialize-skip-completed.db")(_body)
-
-
 async def _materialize_with_change(
     database, start_iso, end_iso, change
 ) -> int: