From b0fcf29f84814fc21f389b88f6b51837f227a613 Mon Sep 17 00:00:00 2001 From: Joshua Date: Thu, 17 Sep 2026 14:22:12 -0400 Subject: [PATCH] feat: regenerate_for_definition and wiring --- custom_components/nestquest/materialize.py | 35 ++++- .../nestquest/quest_definitions.py | 12 +- tests/test_materialize.py | 138 +++++++++++++++++- tests/test_quest_definitions.py | 66 ++++----- 4 files changed, 208 insertions(+), 43 deletions(-) diff --git a/custom_components/nestquest/materialize.py b/custom_components/nestquest/materialize.py index 577533c..a5e5abf 100644 --- a/custom_components/nestquest/materialize.py +++ b/custom_components/nestquest/materialize.py @@ -42,6 +42,7 @@ from __future__ import annotations import datetime +from .const import DEFAULT_HORIZON_DAYS from .dao_instances import QuestInstancesDao from .dao_rules import ( _validate_date, @@ -166,4 +167,36 @@ async def materialize( if written is not None: count += 1 cursor += datetime.timedelta(days=1) - return count \ No newline at end of file + 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) diff --git a/custom_components/nestquest/quest_definitions.py b/custom_components/nestquest/quest_definitions.py index ca38322..817f7fc 100644 --- a/custom_components/nestquest/quest_definitions.py +++ b/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, diff --git a/tests/test_materialize.py b/tests/test_materialize.py index bd5025d..e114678 100644 --- a/tests/test_materialize.py +++ b/tests/test_materialize.py @@ -12,15 +12,25 @@ from custom_components.nestquest.dao_instances import ( 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 +from custom_components.nestquest.recurrence import ( + RuleType, + ScheduleRule, + occurs_on, +) def _run(coro): @@ -642,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) \ No newline at end of file + _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) \ No newline at end of file diff --git a/tests/test_quest_definitions.py b/tests/test_quest_definitions.py index 758a01a..2770113 100644 --- a/tests/test_quest_definitions.py +++ b/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 -- 2.55.0