feat: regenerate_for_definition and wiring

custom_components/nestquest/dao_instances.py

@@ -314,9 +314,13 @@ 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 and the immutable-completed-instance refusal are
-        preserved from :meth:`upsert` (those are not configuration
-        invalidations and still raise ValueError).
+        no-past rule (instances are never generated in the past) is
+        preserved from :meth:`upsert` and still raises ValueError.  An
+        instance that ALREADY has a completion event is also skipped
+        (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)
@@ -359,12 +363,12 @@ class QuestInstancesDao:
                     (definition_id, child_id, due_date, window),
                 )
                 if existing is not None:
-                    raise ValueError(
-                        f"instance for (definition {definition_id}, "
-                        f"child {child_id}, due_date {due_date!r}, "
-                        f"window {window!r}) already has a completion "
-                        "event and is immutable; it cannot be regenerated"
-                    )
+                    # The instance is already completed and therefore
+                    # immutable: the walk SKIPS it (no-op) rather than
+                    # rewriting its snapshot columns or failing the
+                    # batch.  This is the materialization walk's skip,
+                    # distinct from the generic :meth:`upsert` refusal.
+                    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) and skip-completed
-generation are separate later tasks and live elsewhere.
+windows; HA scheduling (day-rollover triggers) is a separate later
+task and lives elsewhere.
 
 The walk is deterministic and idempotent:
 
@@ -32,14 +32,17 @@ 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, window),
-so re-running the materialization over the same range never duplicates a
-row; skipped-completed handling is deliberately NOT done here.
+The upsert is idempotent on (definition_id, child_id, due_date,
+window), so re-running the materialization over the same range never
+duplicates a row; a tuple whose instance is already completed is SKIPPED
+(never rewritten, never raised) by
+:meth:`~.dao_instances.QuestInstancesDao.upsert_if_valid`.
 """
 from __future__ import annotations
 
 import datetime
 
+from .const import DEFAULT_HORIZON_DAYS
 from .dao_instances import QuestInstancesDao
 from .dao_rules import (
     _validate_date,
@@ -164,4 +167,36 @@ async def materialize(
                     if written is not None:
                         count += 1
         cursor += datetime.timedelta(days=1)
-    return count
+    return count
+
+
+async def regenerate_for_definition(
+    database: NestQuestDatabase,
+    definition_id: int,
+) -> int:
+    """Regenerate a definition's future instances after a config change.
+
+    The Feature 06 edit/assignment/activation paths call this after a
+    successful write so future instances track the new state: the
+    definition's open instances at or after today are deleted (never a
+    completed instance, never the past) via
+    :meth:`~.dao_instances.QuestInstancesDao.delete_future_uncompleted`,
+    then the materialization walk re-runs over the rolling horizon
+    ``[today, today + DEFAULT_HORIZON_DAYS]`` so the (changed) rule,
+    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
+    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()
+    start_date = today.isoformat()
+    end_date = (today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS)).isoformat()
+
+    instances = QuestInstancesDao(database)
+    await instances.delete_future_uncompleted(definition_id, start_date)
+    return await materialize(database, start_date, end_date)

custom_components/nestquest/quest_definitions.py

@@ -58,6 +58,7 @@ from .dao_rules import (
     schedule_rule_to_storage,
 )
 from .db import NestQuestDatabase
+from .materialize import regenerate_for_definition
 from .recurrence import ScheduleRule, occurs_on
 
 
@@ -341,6 +342,8 @@ async def edit_quest_definition(
         schedule_rule_storage_from_record(snapshot.rule)
     )
 
+    await regenerate_for_definition(database, definition_id)
+
     return CreatedQuestDefinition(
         definition=snapshot.definition,
         rule=decoded_rule,
@@ -371,7 +374,7 @@ async def assign_child(
     _validate_child_id(child_id)
     dao = QuestDefinitionsDao(database)
     try:
-        return await dao.add_assignee(definition_id, child_id)
+        assignees = await dao.add_assignee(definition_id, child_id)
     except ValueError as error:
         # The DAO names only the child/definition; re-raise so the
         # public contract names the field the caller actually passed.
@@ -381,6 +384,8 @@ async def assign_child(
         if message.startswith("quest definition "):
             raise ValueError(f"definition_id: {message}") from error
         raise
+    await regenerate_for_definition(database, definition_id)
+    return assignees
 
 
 async def unassign_child(
@@ -405,7 +410,9 @@ async def unassign_child(
         raise ValueError(
             f"definition_id: quest definition {definition_id} does not exist"
         )
-    return await dao.remove_assignee(definition_id, child_id)
+    removed = await dao.remove_assignee(definition_id, child_id)
+    await regenerate_for_definition(database, definition_id)
+    return removed
 
 
 async def set_quest_definition_active(
@@ -457,6 +464,7 @@ async def set_quest_definition_active(
     rule = schedule_rule_from_storage(
         schedule_rule_storage_from_record(rule_record)
     )
+    await regenerate_for_definition(database, definition_id)
     return CreatedQuestDefinition(
         definition=updated,
         rule=rule,

tests/test_materialize.py

@@ -7,16 +7,30 @@ import datetime
 import pytest
 
 from custom_components.nestquest.dao_children import ChildrenDao
-from custom_components.nestquest.dao_instances import QuestInstancesDao
+from custom_components.nestquest.dao_instances import (
+    CompletionEventsDao,
+    QuestInstancesDao,
+)
 from custom_components.nestquest.dao_presence import PresenceSchedulesDao
-from custom_components.nestquest.dao_rules import QuestDefinitionsDao
+from custom_components.nestquest.dao_rules import (
+    QuestDefinitionsDao,
+    schedule_rule_to_storage,
+)
 from custom_components.nestquest.db import NestQuestDatabase
-from custom_components.nestquest.materialize import materialize
+from custom_components.nestquest.materialize import (
+    materialize,
+    regenerate_for_definition,
+)
 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,
+    occurs_on,
 )
-from custom_components.nestquest.recurrence import RuleType, ScheduleRule
 
 
 def _run(coro):
@@ -207,15 +221,33 @@ def test_materialize_is_idempotent(tmp_path) -> None:
             [child.id],
             ["morning"],
         )
+        dao = QuestInstancesDao(database)
         first = await materialize(database, start_iso, end_iso)
-        second = await materialize(database, start_iso, end_iso)
-        assert first == second == 7
-        # Running twice refreshes the SAME seven rows: exactly one
-        # physical row per (definition_id, child_id, due_date, window)
-        # key, never a duplicate.
-        records = await QuestInstancesDao(database).list_by_date_range(
-            child.id, start_iso, end_iso
+        assert first == 7
+        first_ids = sorted(
+            r.id
+            for r in await dao.list_by_date_range(child.id, start_iso, end_iso)
         )
+        assert len(first_ids) == 7
+        assert len(set(first_ids)) == 7
+
+        # Re-running over the same range refreshes the SAME physical rows:
+        # the ON CONFLICT path updates the snapshot columns in place and
+        # never delete + reinserts, so every primary-key id is stable.
+        for _ in range(2):
+            count = await materialize(database, start_iso, end_iso)
+            assert count == 7
+            run_ids = sorted(
+                r.id
+                for r in await dao.list_by_date_range(
+                    child.id, start_iso, end_iso
+                )
+            )
+            assert run_ids == first_ids
+
+        # And the widened key never duplicated: exactly one physical row
+        # per (definition_id, child_id, due_date, window).
+        records = await dao.list_by_date_range(child.id, start_iso, end_iso)
         keys = [
             (r.definition_id, r.child_id, r.due_date, r.window)
             for r in records
@@ -227,6 +259,65 @@ 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:
@@ -561,4 +652,124 @@ def test_materialize_skips_inactive_definitions(tmp_path) -> None:
         assert count == 0
         return count
 
-    _with_db(tmp_path, "materialize-inactive.db")(_body)
+    _with_db(tmp_path, "materialize-inactive.db")(_body)
+
+
+def test_regenerate_for_definition_rebuilds_horizon_and_preserves_completed(
+    tmp_path,
+) -> None:
+    async def _body(database):
+        children = ChildrenDao(database)
+        child = await children.create("Ada", NOW)
+
+        today = datetime.date.today()
+        horizon_end = today + datetime.timedelta(days=14)
+        start_iso = today.isoformat()
+
+        created = await create_quest_definition(
+            database,
+            "Daily chore",
+            ScheduleRule(rule_type=RuleType.DAILY, start_date=start_iso),
+            [child.id],
+            [("morning", "09:00")],
+        )
+        definition_id = created.definition.id
+
+        dao = QuestInstancesDao(database)
+        await materialize(database, start_iso, horizon_end.isoformat())
+        baseline = await dao.list_by_date_range(
+            child.id, start_iso, horizon_end.isoformat()
+        )
+        assert len(baseline) == 15
+
+        # Park a completion on tomorrow.  The rule rewrite below gives the
+        # new weekly shape a weekday two days out, so tomorrow never fires
+        # under it — only the completion's immutability keeps the row.
+        completed_due = (today + datetime.timedelta(days=1)).isoformat()
+        completed = next(r for r in baseline if r.due_date == completed_due)
+        await CompletionEventsDao(database).append(
+            completed.id,
+            child.id,
+            "completed",
+            "user",
+            f"{completed_due}T08:00:00+00:00",
+            True,
+            actor_user_id="user-1",
+        )
+
+        # Rewrite the rule through the DAO (not the business layer) so
+        # this test drives ``regenerate_for_definition`` directly.
+        new_weekday = (today + datetime.timedelta(days=2)).weekday()
+        weekly = ScheduleRule(
+            rule_type=RuleType.WEEKLY,
+            weekday_set={new_weekday},
+            start_date=start_iso,
+        )
+        await QuestDefinitionsDao(database).edit_definition(
+            definition_id, rule=schedule_rule_to_storage(weekly)
+        )
+
+        await regenerate_for_definition(database, definition_id)
+
+        records = await dao.list_by_date_range(
+            child.id, start_iso, horizon_end.isoformat()
+        )
+        by_due = {r.due_date: r for r in records}
+
+        # The completed row survived untouched (same physical id).
+        assert by_due[completed_due].id == completed.id
+
+        # Every other horizon day matches the new weekly rule exactly.
+        for offset in range(15):
+            day = today + datetime.timedelta(days=offset)
+            iso = day.isoformat()
+            if iso == completed_due:
+                assert iso in by_due
+            else:
+                assert (iso in by_due) == occurs_on(weekly, day)
+        return None
+
+    _with_db(tmp_path, "regenerate.db")(_body)
+
+
+def test_edit_quest_definition_regenerates_future_instances(tmp_path) -> None:
+    async def _body(database):
+        children = ChildrenDao(database)
+        child = await children.create("Ada", NOW)
+
+        today = datetime.date.today()
+        horizon_end = today + datetime.timedelta(days=14)
+        start_iso = today.isoformat()
+
+        created = await create_quest_definition(
+            database,
+            "Daily chore",
+            ScheduleRule(rule_type=RuleType.DAILY, start_date=start_iso),
+            [child.id],
+            ["morning"],
+        )
+        definition_id = created.definition.id
+
+        dao = QuestInstancesDao(database)
+        await materialize(database, start_iso, horizon_end.isoformat())
+
+        # The business-layer edit wires regeneration: no explicit
+        # ``regenerate_for_definition`` call is made here.
+        new_weekday = (today + datetime.timedelta(days=2)).weekday()
+        weekly = ScheduleRule(
+            rule_type=RuleType.WEEKLY,
+            weekday_set={new_weekday},
+            start_date=start_iso,
+        )
+        await edit_quest_definition(database, definition_id, rule=weekly)
+
+        records = await dao.list_by_date_range(
+            child.id, start_iso, horizon_end.isoformat()
+        )
+        by_due = {r.due_date: r for r in records}
+        for offset in range(15):
+            day = today + datetime.timedelta(days=offset)
+            assert (day.isoformat() in by_due) == occurs_on(weekly, day)
+        return None
+
+    _with_db(tmp_path, "edit-regenerates.db")(_body)

tests/test_quest_definitions.py

@@ -1279,11 +1279,19 @@ def test_set_quest_definition_active_nonexistent_definition_names_field(
     _with_db(tmp_path, "set-active-missing-definition.db")(_body)
 
 
-def test_set_quest_definition_active_never_deletes_or_touches_instances(
+def test_set_quest_definition_active_preserves_completed_and_history(
     tmp_path,
 ) -> None:
-    """Deactivation hides the definition but deletes nothing, and
-    pre-existing instances and completion history survive unchanged."""
+    """Deactivation stops future generation; reactivation resumes it.
+
+    The Feature 07 wiring makes both transitions regenerate the
+    definition's rolling horizon: future OPEN instances are removed on
+    deactivation and re-materialized on reactivation.  A pre-existing
+    COMPLETED instance and its completion events always survive both
+    transitions unchanged (regeneration never touches completed rows or
+    the past), and the append-only completion-events table is never
+    rewritten.
+    """
     async def _body(database, children, rules, definitions, child):
         created = await create_quest_definition(
             database, "Brush teeth", _daily_rule(), [child.id], ["morning"]
@@ -1310,9 +1318,6 @@ def test_set_quest_definition_active_never_deletes_or_touches_instances(
             actor_user_id="admin-1",
         )
 
-        instance_count = await database.fetch_one(
-            "SELECT COUNT(*) FROM quest_instances"
-        )
         event_count = await database.fetch_one(
             "SELECT COUNT(*) FROM completion_events"
         )
@@ -1332,29 +1337,25 @@ def test_set_quest_definition_active_never_deletes_or_touches_instances(
             definition_id
         )] == ["morning"]
         # The pre-existing instance and completion event survive with
-        # unchanged ids and contents, and no new rows appeared.
+        # unchanged ids and contents, and no new event rows appeared.
         assert await instances.get_by_id(instance.id) == instance
         assert await events.list_by_instance(instance.id) == [event]
-        assert await database.fetch_one(
-            "SELECT COUNT(*) FROM quest_instances"
-        ) == instance_count
         assert await database.fetch_one(
             "SELECT COUNT(*) FROM completion_events"
         ) == event_count
 
         await set_quest_definition_active(database, definition_id, True)
         assert (await definitions.get(definition_id)).is_active is True
+        # Reactivation regenerates future OPEN instances, but the
+        # completed instance and its history survive the sweep unchanged.
         assert await instances.get_by_id(instance.id) == instance
         assert await events.list_by_instance(instance.id) == [event]
-        assert await database.fetch_one(
-            "SELECT COUNT(*) FROM quest_instances"
-        ) == instance_count
         assert await database.fetch_one(
             "SELECT COUNT(*) FROM completion_events"
         ) == event_count
         return None
 
-    _with_db(tmp_path, "set-active-never-deletes.db")(_body)
+    _with_db(tmp_path, "set-active-preserves-completed.db")(_body)
 
 
 def _gated_hass(
@@ -1841,16 +1842,15 @@ def test_list_active_definitions_snapshot_coherent_under_racing_edit(
 # ---------------------------------------------------------------------------
 
 
-def test_full_lifecycle_never_touches_instances_or_history(tmp_path) -> None:
-    """End-to-end lifecycle on one definition writes only the four
-    definition tables; ``quest_instances`` and ``completion_events`` are
-    left exactly as they were at every stage.
+def test_full_lifecycle_preserves_completed_and_history(tmp_path) -> None:
+    """End-to-end lifecycle preserves completed rows and append-only history.
 
-    A real instance and completion event are seeded before the lifecycle
-    begins, so each stage is checked against pre-existing data being
-    mutated (not an empty table staying empty): create -> edit
-    rule/windows -> assign a second child -> unassign the original child
-    -> deactivate -> reactivate.
+    Each stage (create -> edit rule/windows -> assign a second child ->
+    unassign the original child -> deactivate -> reactivate) now
+    regenerates the definition's future OPEN instances (Feature 07
+    wiring), but a pre-existing COMPLETED instance and its completion
+    events survive every stage unchanged — the completed row keeps its
+    id and snapshot columns, the completion-events table stays append-only.
     """
     async def _body(database, children, rules, definitions, child):
         created = await create_quest_definition(
@@ -1876,24 +1876,18 @@ def test_full_lifecycle_never_touches_instances_or_history(tmp_path) -> None:
             True,
             actor_user_id="admin-1",
         )
-        instance_count = await database.fetch_one(
-            "SELECT COUNT(*) FROM quest_instances"
-        )
         event_count = await database.fetch_one(
             "SELECT COUNT(*) FROM completion_events"
         )
 
-        async def _assert_history_untouched():
+        async def _assert_history_preserved():
             assert await instances.get_by_id(instance.id) == instance
             assert await events.list_by_instance(instance.id) == [event]
-            assert await database.fetch_one(
-                "SELECT COUNT(*) FROM quest_instances"
-            ) == instance_count
             assert await database.fetch_one(
                 "SELECT COUNT(*) FROM completion_events"
             ) == event_count
 
-        await _assert_history_untouched()
+        await _assert_history_preserved()
 
         # Edit rule and windows together; the definition's rule row is
         # rewritten in place and its window set replaced.
@@ -1914,32 +1908,32 @@ def test_full_lifecycle_never_touches_instances_or_history(tmp_path) -> None:
         )
         by_name = {w.window: w.due_time for w in edited.windows}
         assert by_name == {"morning": "08:00", "evening": "19:30"}
-        await _assert_history_untouched()
+        await _assert_history_preserved()
 
         # Assign a second child: multi-assignee, both present.
         bo = await children.create("Bo", NOW)
         roster = await assign_child(database, definition_id, bo.id)
         assert sorted(c.id for c in roster) == sorted([child.id, bo.id])
-        await _assert_history_untouched()
+        await _assert_history_preserved()
 
         # Unassign the original child: only the link drops.
         assert await unassign_child(database, definition_id, child.id) is True
         assert [
             c.id for c in await definitions.list_assignees(definition_id)
         ] == [bo.id]
-        await _assert_history_untouched()
+        await _assert_history_preserved()
 
         # Deactivate, then reactivate: the definition survives both.
         deactivated = await set_quest_definition_active(
             database, definition_id, False
         )
         assert deactivated.definition.is_active is False
-        await _assert_history_untouched()
+        await _assert_history_preserved()
         reactivated = await set_quest_definition_active(
             database, definition_id, True
         )
         assert reactivated.definition.is_active is True
-        await _assert_history_untouched()
+        await _assert_history_preserved()
 
         return reactivated