From 9836a6f5248f040b4e13098665d872b7e3ee00e1 Mon Sep 17 00:00:00 2001 From: Joshua Date: Thu, 17 Sep 2026 14:01:20 -0400 Subject: [PATCH 01/12] Core generation walk: assignees x windows fan-out with presence (Feature 07, task 1) (#75) --- custom_components/nestquest/dao_instances.py | 111 ++++ custom_components/nestquest/dao_presence.py | 57 +- custom_components/nestquest/dao_rules.py | 60 +- custom_components/nestquest/materialize.py | 167 ++++++ tests/test_materialize.py | 564 +++++++++++++++++++ 5 files changed, 957 insertions(+), 2 deletions(-) create mode 100644 custom_components/nestquest/materialize.py create mode 100644 tests/test_materialize.py diff --git a/custom_components/nestquest/dao_instances.py b/custom_components/nestquest/dao_instances.py index 93437f1..841c1a7 100644 --- a/custom_components/nestquest/dao_instances.py +++ b/custom_components/nestquest/dao_instances.py @@ -277,6 +277,117 @@ class QuestInstancesDao: assert instance is not None return instance + async def upsert_if_valid( + self, + definition_id: int, + child_id: int, + due_date: str, + generated_at: str, + *, + window: str, + due_time: str | None = None, + ) -> QuestInstanceRecord | None: + """Insert one dated instance iff its tuple is still materializable. + + This is the materialization walk's single write path. Unlike the + generic :meth:`upsert`, which validates only existence and + assignment, this method re-validates EVERY materialization + precondition in the SAME connection-lock-held transaction as the + INSERT — closing the check-then-insert gap a concurrent config + write could otherwise slip through: + + - the definition exists AND is active (a definition deactivated + after the input snapshot is skipped, never written); + - the child exists AND is active (a deactivated child is skipped); + - 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). + + 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 + 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). + """ + _validate_date(due_date, "due_date") + _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() + if due_date < today: + raise ValueError( + f"instances are never generated in the past: due_date " + f"{due_date!r} is before today {today!r}" + ) + async with _connection_lock(self._database): + async with self._database.transaction(): + today = datetime.date.today().isoformat() + if due_date < today: + raise ValueError( + f"instances are never generated in the past: " + f"due_date {due_date!r} is before today {today!r}" + ) + 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 " + "JOIN quest_definition_windows w " + "ON w.definition_id = d.id " + "WHERE d.id = ? AND a.child_id = ? AND w.window = ? " + "AND d.is_active = 1 AND c.is_active = 1 LIMIT 1", + (definition_id, child_id, window), + ) + if still_valid is None: + # A config change invalidated the tuple since the + # snapshot: skip it rather than abort the batch. + return None + existing = await self._database.fetch_one( + "SELECT 1 FROM completion_events " + "WHERE instance_id = (SELECT id FROM quest_instances " + "WHERE definition_id = ? AND child_id = ? " + "AND due_date = ? AND window = ?)", + (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" + ) + await self._database.execute( + "INSERT INTO quest_instances (definition_id, child_id, " + "window, due_date, due_time, generated_at) " + "VALUES (?, ?, ?, ?, ?, ?) " + "ON CONFLICT (definition_id, child_id, due_date, " + "window) DO UPDATE SET " + "due_time = excluded.due_time, " + "generated_at = excluded.generated_at", + ( + definition_id, + child_id, + window, + due_date, + due_time, + generated_at, + ), + ) + instance = await self.get( + definition_id, child_id, due_date, window + ) + assert instance is not None + return instance + async def get( self, definition_id: int, diff --git a/custom_components/nestquest/dao_presence.py b/custom_components/nestquest/dao_presence.py index 5279a25..cf1707a 100644 --- a/custom_components/nestquest/dao_presence.py +++ b/custom_components/nestquest/dao_presence.py @@ -276,4 +276,59 @@ class PresenceOverridesDao: "DELETE FROM presence_overrides WHERE id = ?", (override_id,), ) - return result.rowcount > 0 \ No newline at end of file + return result.rowcount > 0 + + +async def read_snapshot_unlocked( + database: NestQuestDatabase, + child_ids: list[int], + range_start: str, + range_end: str, +) -> tuple[ + dict[int, PresenceScheduleRecord], + dict[int, list[PresenceOverrideRecord]], +]: + """Read schedules and overrides for ``child_ids`` WITHOUT locking. + + MUST be called inside the connection lock and an open transaction, so + the schedules and overrides returned here stay coherent with every + other table the caller reads inside that same transaction (the + materialization walk's single input snapshot does exactly that). The + date range comes already validated by the caller. + + Returns ``(schedules, overrides)``: + + - ``schedules`` maps ``child_id`` to its :class:`PresenceScheduleRecord` + for every child that HAS a schedule row. A child absent from the + mapping has no schedule, i.e. is present every day (Feature 05). + - ``overrides`` maps ``child_id`` to that child's overrides overlapping + ``[range_start, range_end]`` (inclusive boundaries), ordered by start + date; a child with no matching override is absent from the mapping. + + ``child_ids`` must name existing children (the caller derives them + from a trusted snapshot, within the same transaction). + """ + children = sorted(set(child_ids)) + if not children: + return {}, {} + placeholders = ",".join("?" for _ in children) + schedule_rows = await database.fetch_all( + f"SELECT {_SCHEDULE_COLUMNS} FROM presence_schedules " + f"WHERE child_id IN ({placeholders})", + tuple(children), + ) + override_rows = await database.fetch_all( + f"SELECT {_OVERRIDE_COLUMNS} FROM presence_overrides " + f"WHERE child_id IN ({placeholders}) AND end_date >= ? " + "AND start_date <= ? ORDER BY start_date", + tuple(children) + (range_start, range_end), + ) + schedules = { + record.child_id: record + for record in map(_schedule_from_row, schedule_rows) + } + overrides: dict[int, list[PresenceOverrideRecord]] = {} + for row in override_rows: + record = _override_from_row(row) + overrides.setdefault(record.child_id, []).append(record) + return schedules, overrides \ No newline at end of file diff --git a/custom_components/nestquest/dao_rules.py b/custom_components/nestquest/dao_rules.py index 5669368..ec3f6d9 100644 --- a/custom_components/nestquest/dao_rules.py +++ b/custom_components/nestquest/dao_rules.py @@ -951,6 +951,21 @@ class QuestDefinitionsDao: definitions = await self.list_active() return await self._snapshots_for(definitions) + async def read_snapshots_active(self) -> list[QuestDefinitionSnapshot]: + """Return every active definition's snapshot WITHOUT locking. + + Mirrors :meth:`list_snapshots_active` minus the lock and + transaction: the definitions and their rules, assignees and + windows are read on the caller's connection under whatever + transaction the caller is already holding, so they stay coherent + with every OTHER table the caller reads inside that same + transaction. MUST be called inside the connection lock and an + open transaction (the single-transaction input snapshot the + materialization walk uses is that caller). + """ + definitions = await self.list_active() + return await self._snapshots_for(definitions) + async def list_snapshots_by_child( self, child_id: int ) -> list[QuestDefinitionSnapshot]: @@ -1189,4 +1204,47 @@ class QuestDefinitionsDao: if not child[0]: raise ValueError( f"child {child_id} is inactive and cannot be assigned" - ) \ No newline at end of file + ) + + +async def load_materialization_input( + database: NestQuestDatabase, + range_start: str, + range_end: str, +): + """Read every materialization input in ONE locked transaction. + + Returns ``(snapshots, schedules, overrides)``: + + - ``snapshots``: every active definition with its rule, assignees and + windows (the :class:`QuestDefinitionSnapshot` shape). + - ``schedules`` / ``overrides``: the presence schedules and + range-scoped overrides of every assignee child, in + :mod:`.dao_presence`'s record shapes. + + The whole read — definitions, rules, assignees, windows, and the + assignees' presence schedules and overrides — runs inside a single + BEGIN..COMMIT span under the connection lock, so a concurrent edit + (definition, assignment, window, or presence) cannot interleave + between the parts and the walk never sees a mixed-time view. Each + table's SQL stays in its own DAO module: the presence portion is + delegated to :mod:`.dao_presence` (a lazy import here, since that + module imports this one). + """ + definitions_dao = QuestDefinitionsDao(database) + async with _connection_lock(database): + async with database.transaction(): + snapshots = await definitions_dao.read_snapshots_active() + child_ids = sorted( + { + child.id + for snapshot in snapshots + for child in snapshot.assignees + } + ) + from . import dao_presence # lazy: dao_presence imports us + + schedules, overrides = await dao_presence.read_snapshot_unlocked( + database, child_ids, range_start, range_end + ) + return snapshots, schedules, overrides \ No newline at end of file diff --git a/custom_components/nestquest/materialize.py b/custom_components/nestquest/materialize.py new file mode 100644 index 0000000..5c4de5b --- /dev/null +++ b/custom_components/nestquest/materialize.py @@ -0,0 +1,167 @@ +"""The materialization walk (Feature 07 core). + +: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. + +The walk is deterministic and idempotent: + +- All inputs — the active definitions with their rules, assignees and + windows, AND the assignee children's presence schedules and scoped + overrides — are read inside ONE locked transaction (see + :func:`~.dao_rules.load_materialization_input`), so the walk sees a + single coherent snapshot rather than a definition state from one + instant and a presence state from another. +- Presence is snapshotted into an immutable + :class:`~.presence.PresenceEngine` from that same coherent read. +- For each date in the range, each definition whose decoded rule + :func:`~.recurrence.occurs_on` on that date, each assignee who is + ``is_present`` on that date and each declared window, one instance is + written through :class:`~.dao_instances.QuestInstancesDao.upsert_if_valid` + with a ``generated_at`` stamp shared by the whole batch (one coherent + time per run). + +A config change that lands AFTER the snapshot is read but BEFORE a +tuple's write (an assignment removed, a definition or child deactivated, +a window removed) makes that tuple SKIP rather than abort the batch: +:meth:`~.dao_instances.QuestInstancesDao.upsert_if_valid` re-validates +every materialization precondition (active definition, active child, +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. +""" +from __future__ import annotations + +import datetime + +from .dao_instances import QuestInstancesDao +from .dao_rules import ( + _validate_date, + load_materialization_input, + schedule_rule_from_storage, + schedule_rule_storage_from_record, +) +from .db import NestQuestDatabase +from .presence import PresenceEngine, PresenceOverride, PresenceSchedule +from .recurrence import ScheduleRule, occurs_on + + +def _now_stamp() -> str: + """Return the batch's shared UTC second-precision generation stamp.""" + return datetime.datetime.now(datetime.timezone.utc).isoformat( + timespec="seconds" + ) + + +def _decode_rule(snapshot) -> ScheduleRule: + """Decode a snapshot's rule record back into a ScheduleRule. + + The record carried by the snapshot is the one the definition + referenced at read time, so decoding it here (rather than re-reading + through ``ScheduleRulesDao``) preserves the coherence the snapshot + guarantees. + """ + return schedule_rule_from_storage( + schedule_rule_storage_from_record(snapshot.rule) + ) + + +def _build_engine( + schedules_records, + overrides_records, +) -> PresenceEngine: + """Convert raw presence records into an immutable PresenceEngine.""" + schedules: dict[int, PresenceSchedule] = {} + for child_id, record in schedules_records.items(): + schedules[child_id] = PresenceSchedule.decode( + child_id, record.anchor_date, record.pattern + ) + overrides: dict[int, list[PresenceOverride]] = {} + for child_id, records in overrides_records.items(): + overrides[child_id] = [ + PresenceOverride( + record.child_id, + record.start_date, + record.end_date, + record.is_present, + record.note, + ) + for record in records + ] + return PresenceEngine(schedules, overrides) + + +async def materialize( + database: NestQuestDatabase, + start_date: str, + end_date: str, +) -> int: + """Generate quest instances for the closed range [start_date, end_date]. + + ``start_date`` and ``end_date`` must be strict ISO calendar dates + (YYYY-MM-DD) with ``end_date`` on or after ``start_date``; anything + else raises ValueError naming the offending field. Returns the number + of instances upserted for the range (idempotent — a re-run refreshes + the same rows, never duplicates them). + + All inputs are read in ONE locked transaction; each tuple's write then + goes through the atomic :meth:`~.dao_instances.QuestInstancesDao.upsert_if_valid`, + which re-validates the tuple and its current ``due_time`` in the same + transaction as the INSERT, so a config change that arrives mid-batch + skips the affected tuples rather than aborting the batch or leaving a + partial result. + """ + _validate_date(start_date, "start_date") + _validate_date(end_date, "end_date") + if end_date < start_date: + raise ValueError( + f"end_date must be on or after start_date, got " + f"{end_date!r} < {start_date!r}" + ) + + snapshots, schedules_records, overrides_records = await ( + load_materialization_input(database, start_date, end_date) + ) + engine = _build_engine(schedules_records, overrides_records) + + definitions = [ + (snapshot.definition.id, _decode_rule(snapshot), + snapshot.assignees, snapshot.windows) + for snapshot in snapshots + ] + + generated_at = _now_stamp() + instances = QuestInstancesDao(database) + + start = datetime.datetime.strptime(start_date, "%Y-%m-%d").date() + end = datetime.datetime.strptime(end_date, "%Y-%m-%d").date() + + count = 0 + cursor = start + while cursor <= end: + iso = cursor.isoformat() + for definition_id, rule, assignees, windows in definitions: + if not occurs_on(rule, cursor): + continue + for child in assignees: + if not engine.is_present(child.id, cursor): + continue + for window in windows: + written = await instances.upsert_if_valid( + definition_id, + child.id, + iso, + generated_at, + window=window.window, + due_time=window.due_time, + ) + if written is not None: + count += 1 + cursor += datetime.timedelta(days=1) + return count \ No newline at end of file diff --git a/tests/test_materialize.py b/tests/test_materialize.py new file mode 100644 index 0000000..86dfe31 --- /dev/null +++ b/tests/test_materialize.py @@ -0,0 +1,564 @@ +"""Tests for materialize.py: the Feature 07 materialization walk.""" +from __future__ import annotations + +import asyncio +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_presence import PresenceSchedulesDao +from custom_components.nestquest.dao_rules import QuestDefinitionsDao +from custom_components.nestquest.db import NestQuestDatabase +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, +) +from custom_components.nestquest.recurrence import RuleType, ScheduleRule + + +def _run(coro): + return asyncio.new_event_loop().run_until_complete(coro) + + +def _make_hass_mock(): + from unittest.mock import AsyncMock, MagicMock + + hass = MagicMock() + hass.async_add_executor_job = AsyncMock(side_effect=(lambda fn, *a: fn(*a))) + return hass + + +NOW = "2026-09-14T12:00:00+00:00" + + +async def _prepare(path): + database = NestQuestDatabase(_make_hass_mock()) + await database.open(path) + await apply_migrations(database) + return database + + +def _with_db(tmp_path, name): + def _run_test(body): + async def _main(): + database = await _prepare(tmp_path / name) + try: + return await body(database) + finally: + await database.close() + + return _run(_main()) + + return _run_test + + +def _future_monday() -> datetime.date: + """The Monday of next week (strictly future, all 28 days eligible).""" + today = datetime.date.today() + this_monday = today - datetime.timedelta(days=today.weekday()) + return this_monday + datetime.timedelta(days=7) + + +def _build_expected(start, def1_id, def2_id, a_id, b_id, c_id) -> set: + """Hand-derived expected (definition_id, child_id, window, due_date, + due_time) tuples over the four-week window. + + Custody: A is present week 0 and 2 of the two-week cycle, absent + 1 and 3; B is the mirror; C has no schedule (present every day). + """ + expected: set = set() + start_dt = datetime.date.fromisoformat(start) + for offset in range(28): + day = start_dt + datetime.timedelta(days=offset) + iso = day.isoformat() + week = offset // 7 + monday = day.weekday() == 0 + + # Def1: DAILY, assignees A + C, window morning due 09:00. + if week % 2 == 0: + expected.add((def1_id, a_id, "morning", iso, "09:00")) + expected.add((def1_id, c_id, "morning", iso, "09:00")) + + # Def2: WEEKLY Monday, assignees B + C, morning 07:00 + evening 19:00. + if monday: + if week % 2 == 1: + expected.add((def2_id, b_id, "morning", iso, "07:00")) + expected.add((def2_id, b_id, "evening", iso, "19:00")) + expected.add((def2_id, c_id, "morning", iso, "07:00")) + expected.add((def2_id, c_id, "evening", iso, "19:00")) + return expected + + +async def _collect_instances(database, child_ids, start, end) -> set: + dao = QuestInstancesDao(database) + collected = set() + for child_id in child_ids: + for record in await dao.list_by_date_range(child_id, start, end): + collected.add( + ( + record.definition_id, + record.child_id, + record.window, + record.due_date, + record.due_time, + ) + ) + return collected + + +def test_materialize_two_custody_one_always_present(tmp_path) -> None: + async def _body(database): + children = ChildrenDao(database) + schedules = PresenceSchedulesDao(database) + + a = await children.create("Ada", NOW) # present weeks 0, 2 + b = await children.create("Bo", NOW) # present weeks 1, 3 + c = await children.create("Cleo", NOW) # no schedule: always present + + start = _future_monday() + start_iso = start.isoformat() + end_iso = (start + datetime.timedelta(days=27)).isoformat() + + await schedules.upsert_by_child( + a.id, 2, start_iso, "0,1,2,3,4,5,6|" + ) + await schedules.upsert_by_child( + b.id, 2, start_iso, "|0,1,2,3,4,5,6" + ) + + def1 = await create_quest_definition( + database, + "Daily chore", + ScheduleRule(rule_type=RuleType.DAILY, start_date=start_iso), + [a.id, c.id], + [("morning", "09:00")], + ) + def2 = await create_quest_definition( + database, + "Weekly chore", + ScheduleRule( + rule_type=RuleType.WEEKLY, + weekday_set={0}, + start_date=start_iso, + ), + [b.id, c.id], + [("morning", "07:00"), ("evening", "19:00")], + ) + + expected = _build_expected( + start_iso, def1.definition.id, def2.definition.id, + a.id, b.id, c.id, + ) + + count = await materialize(database, start_iso, end_iso) + assert count == len(expected) + + actual = await _collect_instances( + database, (a.id, b.id, c.id), start_iso, end_iso + ) + assert actual == expected + return count + + _with_db(tmp_path, "materialize.db")(_body) + + +def test_materialize_shares_one_generated_at_per_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=6)).isoformat() + await create_quest_definition( + database, + "Daily chore", + ScheduleRule(rule_type=RuleType.DAILY, start_date=start_iso), + [child.id], + ["morning"], + ) + await materialize(database, start_iso, end_iso) + dao = QuestInstancesDao(database) + stamps = { + record.generated_at + for record in await dao.list_by_date_range( + child.id, start_iso, end_iso + ) + } + assert len(stamps) == 1 + return stamps + + _with_db(tmp_path, "materialize-stamp.db")(_body) + + +def test_materialize_is_idempotent(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=6)).isoformat() + await create_quest_definition( + database, + "Daily chore", + ScheduleRule(rule_type=RuleType.DAILY, start_date=start_iso), + [child.id], + ["morning"], + ) + 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 + ) + keys = [ + (r.definition_id, r.child_id, r.due_date, r.window) + for r in records + ] + assert len(keys) == 7 + assert len(set(keys)) == 7 + return first + + _with_db(tmp_path, "materialize-idempotent.db")(_body) + + +async def _materialize_with_change( + database, start_iso, end_iso, change +) -> int: + """Run materialize, pause before its first atomic write, apply + ``change``, then release. Returns the materialize count. + + The gate parks the walk AFTER the single input snapshot is read but + BEFORE the first :meth:`QuestInstancesDao.upsert_if_valid` call + acquires the connection lock, so ``change`` commits against the live + state and the write's own in-transaction precondition check then + skips the tuples it invalidated (or, for a presence-only change, + proceeds with the snapped presence). This deterministically + exercises the snapshot-to-insert gap that a config edit can + interleave into. + """ + original_upsert = QuestInstancesDao.upsert_if_valid + entered = asyncio.Event() + release = asyncio.Event() + seen = {"n": 0} + + async def _gated(self, definition_id, child_id, due_date, generated_at, + *, window, due_time=None): + seen["n"] += 1 + if seen["n"] == 1: + seen["first"] = (definition_id, child_id, window) + entered.set() + await release.wait() + return await original_upsert( + self, definition_id, child_id, due_date, generated_at, + window=window, due_time=due_time, + ) + + QuestInstancesDao.upsert_if_valid = _gated + try: + task = asyncio.ensure_future( + materialize(database, start_iso, end_iso) + ) + await entered.wait() + await change() + release.set() + return await task + finally: + QuestInstancesDao.upsert_if_valid = original_upsert + + +def test_materialize_skips_removed_assignee_mid_walk(tmp_path) -> None: + async def _body(database): + children = ChildrenDao(database) + a = await children.create("Ada", NOW) + c = await children.create("Cleo", 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), + [a.id, c.id], + ["morning"], + ) + + async def _remove_assignee(): + await QuestDefinitionsDao(database).remove_assignee( + created.definition.id, a.id + ) + + count = await _materialize_with_change( + database, start_iso, end_iso, _remove_assignee + ) + assert count == 3 # Cleo's three days survive + assert await QuestInstancesDao(database).list_by_date_range( + a.id, start_iso, end_iso + ) == [] + assert len(await QuestInstancesDao(database).list_by_date_range( + c.id, start_iso, end_iso + )) == 3 + return count + + _with_db(tmp_path, "materialize-removed-assignee.db")(_body) + + +def test_materialize_skips_deactivated_definition_mid_walk(tmp_path) -> None: + async def _body(database): + children = ChildrenDao(database) + a = await children.create("Ada", NOW) + c = await children.create("Cleo", 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), + [a.id, c.id], + ["morning"], + ) + + async def _deactivate(): + await QuestDefinitionsDao(database).set_active( + created.definition.id, False + ) + + count = await _materialize_with_change( + database, start_iso, end_iso, _deactivate + ) + assert count == 0 + assert await QuestInstancesDao(database).list_by_date_range( + a.id, start_iso, end_iso + ) == [] + assert await QuestInstancesDao(database).list_by_date_range( + c.id, start_iso, end_iso + ) == [] + return count + + _with_db(tmp_path, "materialize-deactivated-definition.db")(_body) + + +def test_materialize_skips_removed_window_mid_walk(tmp_path) -> None: + async def _body(database): + children = ChildrenDao(database) + a = 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), + [a.id], + [("morning", "09:00")], + ) + + async def _remove_window(): + await QuestDefinitionsDao(database).remove_window( + created.definition.id, "morning" + ) + + count = await _materialize_with_change( + database, start_iso, end_iso, _remove_window + ) + assert count == 0 + assert await QuestInstancesDao(database).list_by_date_range( + a.id, start_iso, end_iso + ) == [] + return count + + _with_db(tmp_path, "materialize-removed-window.db")(_body) + + +def test_materialize_skips_deactivated_child_mid_walk(tmp_path) -> None: + async def _body(database): + children = ChildrenDao(database) + a = await children.create("Ada", NOW) + c = await children.create("Cleo", 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), + [a.id, c.id], + ["morning"], + ) + + async def _deactivate_child(): + await ChildrenDao(database).set_active(a.id, False) + + count = await _materialize_with_change( + database, start_iso, end_iso, _deactivate_child + ) + assert count == 3 # Cleo's three days survive + assert await QuestInstancesDao(database).list_by_date_range( + a.id, start_iso, end_iso + ) == [] + assert len(await QuestInstancesDao(database).list_by_date_range( + c.id, start_iso, end_iso + )) == 3 + return count + + _with_db(tmp_path, "materialize-deactivated-child.db")(_body) + + +def test_materialize_uses_single_presence_snapshot_mid_walk(tmp_path) -> None: + async def _body(database): + from custom_components.nestquest.dao_presence import ( + PresenceOverridesDao, + ) + + children = ChildrenDao(database) + overrides = PresenceOverridesDao(database) + child = await children.create("Ada", NOW) + start = _future_monday() + start_iso = start.isoformat() + end_iso = (start + datetime.timedelta(days=2)).isoformat() + await create_quest_definition( + database, + "Daily chore", + ScheduleRule(rule_type=RuleType.DAILY, start_date=start_iso), + [child.id], + ["morning"], + ) + + async def _add_blocking_override(): + await overrides.create( + child.id, start_iso, end_iso, False, note="away" + ) + + # Presence is read once, inside the input snapshot; a blocking + # override landing AFTER that read must not make the walk panic + # or half-apply it. The walk stays fully governed by the + # snapshot it read: Ada was present every day there, so all + # three days still materialize, deterministically. + count = await _materialize_with_change( + database, start_iso, end_iso, _add_blocking_override + ) + assert count == 3 + assert len(await QuestInstancesDao(database).list_by_date_range( + child.id, start_iso, end_iso + )) == 3 + return count + + _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 ( + PresenceOverridesDao, + ) + + children = ChildrenDao(database) + overrides = PresenceOverridesDao(database) + child = await children.create("Ada", NOW) + start = _future_monday() + start_iso = start.isoformat() + end_iso = (start + datetime.timedelta(days=6)).isoformat() + # No schedule means the child is present every day; a blocking + # override covering the whole week must still suppress every day. + await overrides.create( + child.id, start_iso, end_iso, False, note="away all week" + ) + await create_quest_definition( + database, + "Daily chore", + ScheduleRule(rule_type=RuleType.DAILY, start_date=start_iso), + [child.id], + ["morning"], + ) + count = await materialize(database, start_iso, end_iso) + assert count == 0 + return count + + _with_db(tmp_path, "materialize-override.db")(_body) + + +def test_materialize_rejects_malformed_or_inverted_bounds(tmp_path) -> None: + async def _body(database): + start = (_future_monday()).isoformat() + end = (datetime.date.fromisoformat(start) + + datetime.timedelta(days=1)).isoformat() + for bad in ("not-a-date", "2026-9-7", ""): + with pytest.raises(ValueError, match="start_date"): + await materialize(database, bad, end) + with pytest.raises(ValueError, match="end_date"): + await materialize(database, start, bad) + with pytest.raises(ValueError, match="end_date must be on or after"): + await materialize(database, end, start) + return None + + _with_db(tmp_path, "materialize-bounds.db")(_body) + + +def test_materialize_skips_inactive_definitions(tmp_path) -> None: + async def _body(database): + from custom_components.nestquest.quest_definitions import ( + set_quest_definition_active, + ) + + children = ChildrenDao(database) + child = await children.create("Ada", NOW) + start = _future_monday() + start_iso = start.isoformat() + end_iso = (start + datetime.timedelta(days=6)).isoformat() + created = await create_quest_definition( + database, + "Daily chore", + ScheduleRule(rule_type=RuleType.DAILY, start_date=start_iso), + [child.id], + ["morning"], + ) + await set_quest_definition_active(database, created.definition.id, False) + count = await materialize(database, start_iso, end_iso) + assert count == 0 + return count + + _with_db(tmp_path, "materialize-inactive.db")(_body) \ No newline at end of file From 4544f7839034cc7580a4bde732c5275931661cac Mon Sep 17 00:00:00 2001 From: Joshua Date: Thu, 17 Sep 2026 14:04:02 -0400 Subject: [PATCH 02/12] Three-run idempotency with stable instance ids (Feature 07, task 2) (#76) --- tests/test_materialize.py | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/tests/test_materialize.py b/tests/test_materialize.py index 86dfe31..48c5c06 100644 --- a/tests/test_materialize.py +++ b/tests/test_materialize.py @@ -207,15 +207,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 From 168bb7b2f131385040201e98b1e7e1286f65ca5b Mon Sep 17 00:00:00 2001 From: Joshua Date: Thu, 17 Sep 2026 14:09:42 -0400 Subject: [PATCH 03/12] Skip completed instances during materialization (Feature 07, task 3) (#77) --- custom_components/nestquest/dao_instances.py | 22 ++++--- custom_components/nestquest/materialize.py | 12 ++-- tests/test_materialize.py | 65 +++++++++++++++++++- 3 files changed, 84 insertions(+), 15 deletions(-) diff --git a/custom_components/nestquest/dao_instances.py b/custom_components/nestquest/dao_instances.py index 841c1a7..8c9e852 100644 --- a/custom_components/nestquest/dao_instances.py +++ b/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) " diff --git a/custom_components/nestquest/materialize.py b/custom_components/nestquest/materialize.py index 5c4de5b..577533c 100644 --- a/custom_components/nestquest/materialize.py +++ b/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,9 +32,11 @@ 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 diff --git a/tests/test_materialize.py b/tests/test_materialize.py index 48c5c06..bd5025d 100644 --- a/tests/test_materialize.py +++ b/tests/test_materialize.py @@ -7,7 +7,10 @@ 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.db import NestQuestDatabase @@ -15,6 +18,7 @@ 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 @@ -245,6 +249,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: From 21465316657f9128ed8052cf7c10b146b666900a Mon Sep 17 00:00:00 2001 From: Joshua Date: Thu, 17 Sep 2026 14:24:22 -0400 Subject: [PATCH 04/12] Regenerate future instances on definition change (Feature 07, task 4) (#78) --- 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 From ca26ced59f08be75906e420d9c832ad580fea5be Mon Sep 17 00:00:00 2001 From: Joshua Date: Thu, 17 Sep 2026 14:50:50 -0400 Subject: [PATCH 05/12] Regenerate future instances on presence change (Feature 07, task 5) (#79) --- custom_components/nestquest/dao_instances.py | 65 ++++- custom_components/nestquest/materialize.py | 106 +++++++- tests/test_materialize.py | 270 ++++++++++++++++++- 3 files changed, 432 insertions(+), 9 deletions(-) diff --git a/custom_components/nestquest/dao_instances.py b/custom_components/nestquest/dao_instances.py index 8c9e852..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 = ? " @@ -487,6 +499,49 @@ class QuestInstancesDao: ) return result.rowcount + async def delete_future_uncompleted_for_child( + self, child_id: int, cutoff_date: str + ) -> tuple[int, str]: + """Delete the child's open instances at/after cutoff, across every + definition, returning ``(deleted, effective_cutoff)``. + + Child-scoped counterpart to :meth:`delete_future_uncompleted`, + used when a child's presence changes (Feature 09): only instances + at or after the cutoff with NO completion event are removed, inside + one transaction — an instance that gains a completion event + 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 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): + async with self._database.transaction(): + # "today" is read under the lock, matching the + # 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 = _today().isoformat() + effective_cutoff = max(cutoff_date, today) + result = await self._database.execute( + "DELETE FROM quest_instances WHERE child_id = ? " + "AND due_date >= ? AND NOT EXISTS (" + " SELECT 1 FROM completion_events " + " WHERE instance_id = quest_instances.id" + ")", + (child_id, effective_cutoff), + ) + return result.rowcount, effective_cutoff + class CompletionEventsDao: """Typed append-only access to the ``completion_events`` table. diff --git a/custom_components/nestquest/materialize.py b/custom_components/nestquest/materialize.py index a5e5abf..342e626 100644 --- a/custom_components/nestquest/materialize.py +++ b/custom_components/nestquest/materialize.py @@ -37,13 +37,34 @@ 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`. + +Regeneration: a config or presence change must rebuild future instances +so they track the new state. :func:`regenerate_for_definition` covers +definition edits / reassignment (Features 06/07); :func:`regenerate_for_child` +covers a single child's presence change (Feature 09). Both delete the +affected open instances at or after today — never a completed instance, +never the past — and re-run the walk over the rolling horizon +``[today, today + DEFAULT_HORIZON_DAYS]``. + +HOOK (Feature 09 presence services): the presence business layer that +lands in Feature 09 MUST call :func:`regenerate_for_child` after EVERY +presence-state write so a child's future instances track its presence: +``set_presence_pattern`` (including clearing/setting an empty pattern), +deleting/clearing the presence schedule +(:meth:`~.dao_presence.PresenceSchedulesDao.delete`), +``create_presence_override``, and ``delete_presence_override``. A +schedule delete changes presence state too (the child becomes +always-present), so it must regenerate just like the others. This hook +is documented, not yet wired: no presence business layer exists yet, and +the presence DAO (``dao_presence.py``) must stay free of business rules +— never call :func:`regenerate_for_child` from inside the DAO layer. """ 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, @@ -104,6 +125,8 @@ async def materialize( database: NestQuestDatabase, start_date: str, end_date: str, + *, + child_ids: list[int] | None = None, ) -> int: """Generate quest instances for the closed range [start_date, end_date]. @@ -113,6 +136,12 @@ async def materialize( of instances upserted for the range (idempotent — a re-run refreshes the same rows, never duplicates them). + ``child_ids`` optionally scopes the walk to exactly those children: + assignees outside the list are never written, so a caller that only + wants one child's instances rebuilt (e.g. :func:`regenerate_for_child` + after a presence change) cannot touch any unrelated child's rows. + When omitted (or None) the walk covers every assignee as before. + All inputs are read in ONE locked transaction; each tuple's write then goes through the atomic :meth:`~.dao_instances.QuestInstancesDao.upsert_if_valid`, which re-validates the tuple and its current ``due_time`` in the same @@ -132,6 +161,7 @@ async def materialize( load_materialization_input(database, start_date, end_date) ) engine = _build_engine(schedules_records, overrides_records) + child_filter = None if child_ids is None else set(child_ids) definitions = [ (snapshot.definition.id, _decode_rule(snapshot), @@ -153,6 +183,8 @@ async def materialize( if not occurs_on(rule, cursor): continue for child in assignees: + if child_filter is not None and child.id not in child_filter: + continue if not engine.is_present(child.id, cursor): continue for window in windows: @@ -186,17 +218,85 @@ 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() instances = QuestInstancesDao(database) await instances.delete_future_uncompleted(definition_id, start_date) return await materialize(database, start_date, end_date) + + +# HOOK (Feature 09 presence services): ``regenerate_for_child`` is the +# child-presence counterpart to ``regenerate_for_definition``. The +# presence business layer that lands in Feature 09 MUST call +# ``regenerate_for_child`` after EVERY presence-state write, so future +# instances track a child's changed presence: set_presence_pattern +# (including clearing/setting an empty pattern), deleting/clearing the +# presence schedule (PresenceSchedulesDao.delete), +# create_presence_override, and delete_presence_override. A schedule +# delete changes presence state too (the child becomes always-present), +# so it must regenerate just like the others. It is deliberately NOT +# wired into ``dao_presence.py``: the DAO layer stays pure storage (no +# business rules), and no presence business layer exists yet to host the +# call — see the module docstring note above. +async def regenerate_for_child( + database: NestQuestDatabase, + child_id: int, +) -> int: + """Regenerate a child's future instances after a presence change. + + The Feature 09 presence services call this after every + presence-state write — set_presence_pattern (including clearing or + setting an empty pattern), deleting/clearing the presence schedule + (:meth:`~.dao_presence.PresenceSchedulesDao.delete`), + create_presence_override, and delete_presence_override — so a + child's future instances track its new presence: the child's open + instances at or after today are deleted across ALL its definitions + (never a completed instance, never the past) via + :meth:`~.dao_instances.QuestInstancesDao.delete_future_uncompleted_for_child`, + then the materialization walk re-runs over the rolling horizon + ``[today, today + DEFAULT_HORIZON_DAYS]`` scoped to that child only, + so the child's now-absent/present dates re-materialize against + today's presence WITHOUT touching any other child's rows. + + ``child_id`` must be a plain int — bool and float are rejected + (SQLite binds a bool as 0/1 and a float would round, so a malformed + id must never mutate the wrong profile). + + "today" is computed the same way the walk computes it — + :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 for + the child across the whole snapshot. + """ + if isinstance(child_id, bool) or not isinstance(child_id, int): + raise ValueError(f"child_id must be an integer, got {child_id!r}") + + today = _today() + + instances = QuestInstancesDao(database) + # 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, child_ids=[child_id] + ) diff --git a/tests/test_materialize.py b/tests/test_materialize.py index e114678..62523fd 100644 --- a/tests/test_materialize.py +++ b/tests/test_materialize.py @@ -19,6 +19,7 @@ from custom_components.nestquest.dao_rules import ( from custom_components.nestquest.db import NestQuestDatabase from custom_components.nestquest.materialize import ( materialize, + regenerate_for_child, regenerate_for_definition, ) from custom_components.nestquest.migrations import apply_migrations @@ -772,4 +773,271 @@ def test_edit_quest_definition_regenerates_future_instances(tmp_path) -> None: 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 + _with_db(tmp_path, "edit-regenerates.db")(_body) + + +def test_regenerate_for_child_rebuilds_horizon_preserves_history( + tmp_path, +) -> None: + async def _body(database): + from custom_components.nestquest.dao_presence import ( + PresenceOverridesDao, + ) + + children = ChildrenDao(database) + overrides = PresenceOverridesDao(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")], + ) + + dao = QuestInstancesDao(database) + events = CompletionEventsDao(database) + + # Instances across the full horizon: 15 daily rows. + await materialize(database, start_iso, horizon_end.isoformat()) + all_future = await dao.list_by_date_range( + child.id, start_iso, horizon_end.isoformat() + ) + assert len(all_future) == 15 + + # A completed instance on tomorrow: immutable, must be spared. + completed_due = (today + datetime.timedelta(days=1)).isoformat() + completed = next(r for r in all_future if r.due_date == completed_due) + await events.append( + completed.id, + child.id, + "completed", + "user", + f"{completed_due}T08:00:00+00:00", + True, + actor_user_id="user-1", + ) + + # A past instance (yesterday): raw insert bypasses the walk's + # no-past guard so the child-scoped delete has history to spare. + past_due = (today - datetime.timedelta(days=1)).isoformat() + await database.execute( + "INSERT INTO quest_instances (definition_id, child_id, " + "window, due_date, due_time, generated_at) " + "VALUES (?, ?, 'morning', ?, '09:00', ?)", + (created.definition.id, child.id, past_due, NOW), + ) + + # An ABSENT override covering three future dates that had + # instances: days 2..4 after today. + absent_start = (today + datetime.timedelta(days=2)).isoformat() + absent_end = (today + datetime.timedelta(days=4)).isoformat() + override = await overrides.create( + child.id, absent_start, absent_end, False, note="away" + ) + + await regenerate_for_child(database, child.id) + + records = await dao.list_by_date_range( + child.id, past_due, horizon_end.isoformat() + ) + by_due = {r.due_date: r for r in records} + + # The now-absent days' instances are gone. + for offset in range(2, 5): + iso = (today + datetime.timedelta(days=offset)).isoformat() + assert iso not in by_due + + # The completed instance survived untouched (same physical id). + assert by_due[completed_due].id == completed.id + + # The past instance survived. + assert past_due in by_due + + # Every non-absent future day still has its instance. + for offset in range(15): + iso = (today + datetime.timedelta(days=offset)).isoformat() + if 2 <= offset <= 4: + continue + assert iso in by_due + + # Remove the override and regenerate again: the absent days return. + await overrides.delete(override.id) + await regenerate_for_child(database, child.id) + + records = await dao.list_by_date_range( + child.id, past_due, horizon_end.isoformat() + ) + by_due = {r.due_date: r for r in records} + + for offset in range(15): + iso = (today + datetime.timedelta(days=offset)).isoformat() + assert iso in by_due + + # The completed instance is still the same untouched row, and the + # past instance is still there. + assert by_due[completed_due].id == completed.id + assert by_due[completed_due].due_time == "09:00" + assert past_due in by_due + return None + + _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) + + +def test_regenerate_for_child_is_child_scoped(tmp_path) -> None: + """Regenerating one child must never touch another child's rows.""" + async def _body(database): + from custom_components.nestquest.dao_presence import ( + PresenceOverridesDao, + ) + + children = ChildrenDao(database) + overrides = PresenceOverridesDao(database) + a = await children.create("Ada", NOW) + b = await children.create("Bo", NOW) + + today = datetime.date.today() + horizon_end = today + datetime.timedelta(days=14) + start_iso = today.isoformat() + + await create_quest_definition( + database, + "Daily chore", + ScheduleRule(rule_type=RuleType.DAILY, start_date=start_iso), + [a.id, b.id], + [("morning", "09:00")], + ) + + dao = QuestInstancesDao(database) + await materialize(database, start_iso, horizon_end.isoformat()) + + # Baseline for child b, keyed by due_date, so we can prove its rows + # are byte-for-byte unchanged afterward. + baseline_b = { + r.due_date: (r.id, r.due_time, r.generated_at) + for r in await dao.list_by_date_range( + b.id, start_iso, horizon_end.isoformat() + ) + } + assert len(baseline_b) == 15 + + # An ABSENT override for child a only, covering days 2..4. + absent_start = (today + datetime.timedelta(days=2)).isoformat() + absent_end = (today + datetime.timedelta(days=4)).isoformat() + await overrides.create( + a.id, absent_start, absent_end, False, note="away" + ) + + await regenerate_for_child(database, a.id) + + # Child a's now-absent days vanished. + a_by_due = { + r.due_date: r + for r in await dao.list_by_date_range( + a.id, start_iso, horizon_end.isoformat() + ) + } + for offset in range(2, 5): + iso = (today + datetime.timedelta(days=offset)).isoformat() + assert iso not in a_by_due + + # Child b is untouched: same physical rows, same snapshot columns. + b_records = await dao.list_by_date_range( + b.id, start_iso, horizon_end.isoformat() + ) + assert len(b_records) == 15 + for r in b_records: + assert (r.id, r.due_time, r.generated_at) == baseline_b[r.due_date] + return None + + _with_db(tmp_path, "regenerate-child-scoped.db")(_body) + + +def test_regenerate_for_child_rejects_non_int_child_id(tmp_path) -> None: + async def _body(database): + for bad in (True, False, 1.0, "1"): + with pytest.raises(ValueError, match="child_id must be an integer"): + await regenerate_for_child(database, bad) + return None + + _with_db(tmp_path, "regenerate-child-badid.db")(_body) \ No newline at end of file From be6367b600d7aa8fed7d1c07fc85db1e13a3c536 Mon Sep 17 00:00:00 2001 From: Joshua Date: Thu, 17 Sep 2026 15:10:57 -0400 Subject: [PATCH 06/12] Schedule the daily generation run at rollover (Feature 07, task 6) (#80) --- custom_components/nestquest/__init__.py | 87 ++++++++++++++-- tests/conftest.py | 69 +++++++++++++ tests/test_lifecycle.py | 128 +++++++++++++++++++++++- 3 files changed, 276 insertions(+), 8 deletions(-) diff --git a/custom_components/nestquest/__init__.py b/custom_components/nestquest/__init__.py index 91a962d..52a5eeb 100644 --- a/custom_components/nestquest/__init__.py +++ b/custom_components/nestquest/__init__.py @@ -2,19 +2,30 @@ from __future__ import annotations import asyncio +import datetime import inspect import sqlite3 from collections.abc import Callable from dataclasses import dataclass from pathlib import Path from typing import Any +from zoneinfo import ZoneInfo from homeassistant.config_entries import ConfigEntry from homeassistant.core import HomeAssistant from homeassistant.exceptions import ConfigEntryNotReady +from homeassistant.helpers.event import async_track_time_change -from .const import CONF_ADMIN_USER_IDS, DOMAIN, LOGGER +from .const import ( + CONF_ADMIN_USER_IDS, + CONF_DAY_ROLLOVER_TIME, + DEFAULT_DAY_ROLLOVER_TIME, + DEFAULT_HORIZON_DAYS, + DOMAIN, + LOGGER, +) from .db import NestQuestDatabase +from .materialize import materialize as _materialize_run from .migrations import apply_migrations from .store import async_get_db_path from .admin_allowlist import seed_setup_admin @@ -224,6 +235,7 @@ class NestQuestRuntimeData: options: dict[str, Any] database: NestQuestDatabase remove_update_listener: Callable[[], Any] + remove_time_change_listener: Callable[[], Any] async def _async_owner_user_ids(hass: HomeAssistant) -> list[str]: @@ -248,6 +260,48 @@ async def _async_owner_user_ids(hass: HomeAssistant) -> list[str]: return owners +def _day_rollover_hour_minute(options: dict[str, Any]) -> tuple[int, int]: + """Return the (hour, minute) the daily rollover fires at from ``options``. + + The options flow validates ``day_rollover_time`` as a strict ``HH:MM`` + string, so a stored value never needs re-validation here; a missing key + falls back to :data:`~.const.DEFAULT_DAY_ROLLOVER_TIME`. + """ + rollover = options.get(CONF_DAY_ROLLOVER_TIME, DEFAULT_DAY_ROLLOVER_TIME) + hour, _, minute = rollover.partition(":") + return int(hour), int(minute) + + +def _register_day_rollover_listener( + hass: HomeAssistant, + database: NestQuestDatabase, + options: dict[str, Any], +) -> Callable[[], Any]: + """Register the daily materialization listener and return its remover. + + Fires once per day at the configured local ``day_rollover_time`` and + materializes the rolling horizon ``[today, today + DEFAULT_HORIZON_DAYS]``. + "today" is computed in HA local time (``hass.config.time_zone``) rather + than the system clock, so the horizon tracks the household's own day. + The walk itself runs through :func:`~.materialize.materialize`, whose DB + access is executor-only. + """ + hour, minute = _day_rollover_hour_minute(options) + + async def _run_materialization(_now: datetime.datetime) -> None: + time_zone = ZoneInfo(hass.config.time_zone) + today = datetime.datetime.now(time_zone).date() + start_date = today.isoformat() + end_date = ( + today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS) + ).isoformat() + await _materialize_run(database, start_date, end_date) + + return async_track_time_change( + hass, _run_materialization, hour=hour, minute=minute + ) + + async def async_setup(hass: HomeAssistant, config: dict[str, Any]) -> bool: """Set up the NestQuest integration. YAML configuration is not used, returns True.""" return True @@ -330,6 +384,9 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: owner_ids=owner_ids, ) remove_update_listener = entry.add_update_listener(_async_update_listener) + remove_time_change_listener = _register_day_rollover_listener( + hass, database, dict(entry.options) + ) except BaseException: await database.close() raise @@ -338,6 +395,7 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: options=dict(entry.options), database=database, remove_update_listener=remove_update_listener, + remove_time_change_listener=remove_time_change_listener, ) entry.runtime_data = runtime_data hass.data[DOMAIN][entry.entry_id] = runtime_data @@ -354,14 +412,29 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: runtime_data = getattr(entry, "runtime_data", None) if runtime_data is not None: remove_update_listener = getattr(runtime_data, "remove_update_listener", None) + remove_time_change_listener = getattr( + runtime_data, "remove_time_change_listener", None + ) unload_error: BaseException | None = None try: - if remove_update_listener is not None: - result = remove_update_listener() - if inspect.isawaitable(result): - await result - except BaseException as err: - unload_error = err + for remove_listener in ( + remove_update_listener, + remove_time_change_listener, + ): + if remove_listener is None: + continue + try: + result = remove_listener() + if inspect.isawaitable(result): + await result + except BaseException as err: + # Cancel EVERY listener even when an earlier remover + # raises: a skipped remover would leave a daily + # callback firing against a closed database after + # unload. The first error is retained and re-raised + # only after both removers (and the DB close) ran. + if unload_error is None: + unload_error = err finally: database = getattr(runtime_data, "database", None) if database is not None: diff --git a/tests/conftest.py b/tests/conftest.py index 5694471..ff8c5d6 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -44,6 +44,7 @@ _HA_MODULES = ( "homeassistant.exceptions", "homeassistant.helpers", "homeassistant.helpers.config_validation", + "homeassistant.helpers.event", ) def _find_spec(name: str): @@ -83,6 +84,7 @@ for _parent, _child in ( ("homeassistant", "exceptions"), ("homeassistant", "helpers"), ("homeassistant.helpers", "config_validation"), + ("homeassistant.helpers", "event"), ): setattr(sys.modules[_parent], _child, sys.modules[f"{_parent}.{_child}"]) @@ -242,6 +244,69 @@ class ListenerRegistry: return sum(len(listeners) for listeners in self._listeners.values()) +class TimeChangeRegistry: + """Models HA's time-change listener bookkeeping. + + ``track`` mirrors ``homeassistant.helpers.event.async_track_time_change``: + each call appends a record (action + hour/minute/second) and returns + a remove callable that reverses the registration. Tests inspect + ``registrations`` and exercise the newest action through :meth:`fire`. + """ + + def __init__(self, hass=None): + self._hass = hass + self._registrations: list[dict] = [] + + def track(self, action, hour=None, minute=None, second=None): + record = { + "action": action, + "hour": hour, + "minute": minute, + "second": second, + } + + def _remove(): + self._registrations.remove(record) + + record["remove"] = _remove + self._registrations.append(record) + return _remove + + @property + def registrations(self): + """Return a copy of the live registrations.""" + return list(self._registrations) + + @property + def size(self) -> int: + """Number of live registrations.""" + return len(self._registrations) + + async def fire(self, now=None, index: int = -1): + """Run a registered action as HA's scheduler would, awaiting coroutines.""" + result = self._registrations[index]["action"](now) + if inspect.isawaitable(result): + await result + + +def _async_track_time_change(hass, action, hour=None, minute=None, second=None): + """Mirror homeassistant.helpers.event.async_track_time_change. + + Exact HA 2024.6 signature — no ``local`` keyword: the local-time + wrapper is already this function. Routes to the registry attached to + ``hass`` (created on demand), so tests reach the registrations via + ``hass.time_change``. + """ + registry = getattr(hass, "time_change", None) + if registry is None: + registry = TimeChangeRegistry(hass) + hass.time_change = registry + return registry.track(action, hour=hour, minute=minute, second=second) + + +_ha_mock("homeassistant.helpers.event").async_track_time_change = _async_track_time_change + + def make_config_entry( entry_id: str = "test_entry", options: dict | None = None, @@ -303,6 +368,10 @@ def make_hass() -> tuple: hass.async_add_executor_job = _run_executor_job registry = ListenerRegistry(hass) + hass.time_change = TimeChangeRegistry(hass) + # A valid IANA time zone (the day-rollover listener reads it to + # compute "today" in HA local time). + hass.config.time_zone = "UTC" async def _async_reload(entry_id: str) -> None: registry.reloaded.append(entry_id) diff --git a/tests/test_lifecycle.py b/tests/test_lifecycle.py index c3e0f17..f56864f 100644 --- a/tests/test_lifecycle.py +++ b/tests/test_lifecycle.py @@ -8,7 +8,11 @@ import pytest from conftest import make_config_entry, make_hass, wire_entry_to_registry from custom_components.nestquest import DOMAIN, async_setup_entry, async_unload_entry -from custom_components.nestquest.const import DOMAIN as DOMAIN_CONST +from custom_components.nestquest.const import ( + CONF_DAY_ROLLOVER_TIME, + DEFAULT_HORIZON_DAYS, + DOMAIN as DOMAIN_CONST, +) def _wire(entry, registry): @@ -237,6 +241,128 @@ async def test_unload_closes_database_even_when_listener_removal_raises() -> Non assert database.connected is False, "database leaked despite remover error" assert entry.runtime_data is None assert DOMAIN not in hass.data + assert hass.time_change.size == 0, "time-change listener leaked" + + +async def test_unload_cancels_time_listener_even_when_update_remover_raises() -> None: + """A raising update-listener remover must not skip the time-change cancel. + + Both removers are independent: the update-listener remover raising must + still cancel the daily time-change callback, whose action would otherwise + fire after unload against a closed database. + """ + + class _RemoveBoom(Exception): + pass + + def _remover(): + raise _RemoveBoom() + + entry = make_config_entry(entry_id="update-boom-time-cancel") + entry.add_update_listener = lambda listener: _remover + hass, _registry = make_hass() + await async_setup_entry(hass, entry) + assert hass.time_change.size == 1 + with pytest.raises(_RemoveBoom): + await async_unload_entry(hass, entry) + assert hass.time_change.size == 0, "time-change listener not cancelled" + assert entry.runtime_data is None + assert DOMAIN not in hass.data + + +async def test_setup_registers_day_rollover_listener(hass, make_entry) -> None: + """Setup registers a daily rollover listener and stores its cancel fn.""" + entry = _wire(make_entry(), hass.registry) + assert await async_setup_entry(hass, entry) is True + assert hass.time_change.size == 1 + registration = hass.time_change.registrations[0] + assert registration["hour"] == 0 + assert registration["minute"] == 0 + assert callable(entry.runtime_data.remove_time_change_listener) + + +async def test_setup_registers_day_rollover_at_configured_time(hass, make_entry) -> None: + """The listener fires at the entry option's rollover time, not the default.""" + entry = _wire(make_entry(options={CONF_DAY_ROLLOVER_TIME: "06:30"}), hass.registry) + await async_setup_entry(hass, entry) + registration = hass.time_change.registrations[0] + assert registration["hour"] == 6 + assert registration["minute"] == 30 + + +async def test_unload_cancels_day_rollover_listener(hass, make_entry) -> None: + """Unload removes the time-change listener alongside the update listener.""" + entry = _wire(make_entry(), hass.registry) + await async_setup_entry(hass, entry) + assert hass.time_change.size == 1 + assert await async_unload_entry(hass, entry) is True + assert hass.time_change.size == 0 + + +async def test_rollover_time_change_reregisters_listener_at_new_time( + hass, make_entry +) -> None: + """A rollover-time options change re-registers the listener at the new time.""" + entry = _wire( + make_entry(options={CONF_DAY_ROLLOVER_TIME: "00:00"}), hass.registry + ) + await async_setup_entry(hass, entry) + assert (hass.time_change.registrations[0]["hour"], hass.time_change.registrations[0]["minute"]) == (0, 0) + + # The options flow writes the new rollover time, then HA reloads the + # entry: unload tears down the old listener, setup re-registers it. + entry.options = {CONF_DAY_ROLLOVER_TIME: "06:30"} + await async_unload_entry(hass, entry) + assert hass.time_change.size == 0 + + assert await async_setup_entry(hass, entry) is True + assert hass.time_change.size == 1 + registration = hass.time_change.registrations[0] + assert (registration["hour"], registration["minute"]) == (6, 30) + + +async def test_day_rollover_listener_materializes_ha_local_horizon( + hass, make_entry, monkeypatch +) -> None: + """Firing the listener materializes today..today+horizon in HA local time.""" + import datetime + from unittest.mock import AsyncMock + from zoneinfo import ZoneInfo + + import custom_components.nestquest as nestquest + + entry = _wire(make_entry(), hass.registry) + await async_setup_entry(hass, entry) + database = entry.runtime_data.database + + fake = AsyncMock() + monkeypatch.setattr(nestquest, "_materialize_run", fake) + + time_zone = ZoneInfo(hass.config.time_zone) + today = datetime.datetime.now(time_zone).date() + expected_start = today.isoformat() + expected_end = (today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS)).isoformat() + + await hass.time_change.fire() + + fake.assert_awaited_once_with(database, expected_start, expected_end) + + +def test_day_rollover_time_change_tracker_rejects_local_kwarg() -> None: + """The fake async_track_time_change mirrors HA 2024.6 (no ``local`` kwarg). + + Setup drives the local-time wrapper directly, so passing ``local=`` must + raise TypeError — otherwise a regression in the production call would be + silently masked by the fake. + """ + import inspect as _inspect + + from homeassistant.helpers.event import async_track_time_change + + params = _inspect.signature(async_track_time_change).parameters + assert list(params) == ["hass", "action", "hour", "minute", "second"] + with pytest.raises(TypeError): + async_track_time_change(None, lambda _now: None, hour=0, minute=0, local=True) async def test_setup_applies_all_v1_tables(hass, make_entry) -> None: From 5706d8b7a6ca41b40735fe098f41c0452d6856bd Mon Sep 17 00:00:00 2001 From: Joshua Date: Thu, 17 Sep 2026 15:42:10 -0400 Subject: [PATCH 07/12] Startup backfill + regenerate service (Feature 07, task 7) (#81) --- custom_components/nestquest/__init__.py | 132 ++++++++++++-- custom_components/nestquest/const.py | 3 + custom_components/nestquest/dao_instances.py | 104 +++++++---- custom_components/nestquest/materialize.py | 48 +++-- custom_components/nestquest/services.yaml | 12 ++ tests/conftest.py | 45 +++++ tests/test_lifecycle.py | 176 ++++++++++++++++++- tests/test_materialize.py | 42 ++++- tests/test_services.py | 41 +++++ 9 files changed, 529 insertions(+), 74 deletions(-) create mode 100644 custom_components/nestquest/services.yaml create mode 100644 tests/test_services.py diff --git a/custom_components/nestquest/__init__.py b/custom_components/nestquest/__init__.py index 52a5eeb..6615679 100644 --- a/custom_components/nestquest/__init__.py +++ b/custom_components/nestquest/__init__.py @@ -23,6 +23,7 @@ from .const import ( DEFAULT_HORIZON_DAYS, DOMAIN, LOGGER, + SERVICE_REGENERATE, ) from .db import NestQuestDatabase from .materialize import materialize as _materialize_run @@ -272,6 +273,29 @@ def _day_rollover_hour_minute(options: dict[str, Any]) -> tuple[int, int]: return int(hour), int(minute) +async def _run_horizon_materialization( + hass: HomeAssistant, database: NestQuestDatabase +) -> None: + """Materialize the rolling horizon ``[today, today + DEFAULT_HORIZON_DAYS]``. + + "today" is computed in HA local time (``hass.config.time_zone``) rather + than the system clock, so the horizon tracks the household's own day. + The walk itself runs through :func:`~.materialize.materialize`, whose DB + access is executor-only. + + This is the ONE generation path: the startup backfill, the daily rollover + listener and the ``regenerate`` service all funnel through it, so there is + no duplicated materialization logic. + """ + time_zone = ZoneInfo(hass.config.time_zone) + today = datetime.datetime.now(time_zone).date() + start_date = today.isoformat() + end_date = ( + today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS) + ).isoformat() + await _materialize_run(database, start_date, end_date, today=today) + + def _register_day_rollover_listener( hass: HomeAssistant, database: NestQuestDatabase, @@ -280,28 +304,69 @@ def _register_day_rollover_listener( """Register the daily materialization listener and return its remover. Fires once per day at the configured local ``day_rollover_time`` and - materializes the rolling horizon ``[today, today + DEFAULT_HORIZON_DAYS]``. - "today" is computed in HA local time (``hass.config.time_zone``) rather - than the system clock, so the horizon tracks the household's own day. - The walk itself runs through :func:`~.materialize.materialize`, whose DB - access is executor-only. + materializes the rolling horizon ``[today, today + DEFAULT_HORIZON_DAYS]`` + through the shared :func:`_run_horizon_materialization` path. """ hour, minute = _day_rollover_hour_minute(options) async def _run_materialization(_now: datetime.datetime) -> None: - time_zone = ZoneInfo(hass.config.time_zone) - today = datetime.datetime.now(time_zone).date() - start_date = today.isoformat() - end_date = ( - today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS) - ).isoformat() - await _materialize_run(database, start_date, end_date) + await _run_horizon_materialization(hass, database) return async_track_time_change( hass, _run_materialization, hour=hour, minute=minute ) +def _find_live_database(hass: HomeAssistant) -> NestQuestDatabase | None: + """Return a connected database from the active runtime data, or None. + + The domain-global services (e.g. ``regenerate``) have no database of + their own; they resolve whichever config entry's database is currently + live at call time, so none of them can hold a stale handle to a closed + connection after another entry unloads. + """ + for runtime_data in hass.data.get(DOMAIN, {}).values(): + database = getattr(runtime_data, "database", None) + if database is not None and database.connected: + return database + return None + + +def _register_regenerate_service(hass: HomeAssistant) -> None: + """Register ``nestquest.regenerate`` once at domain scope. + + The service triggers a full on-demand regeneration over the rolling + horizon, running the SAME idempotent materialize path + (:func:`_run_horizon_materialization`) as the startup backfill and the + daily rollover listener. It is DOMAIN-global — registered exactly once + (guarded by ``has_service``) and NOT bound to any config entry — so the + handler resolves the currently-live database at call time and a name + collision or duplicate setup is impossible. Removal is handled by the + unload path only once the LAST entry is removed (see + :func:`_deregister_regenerate_service`). + """ + if hass.services.has_service(DOMAIN, SERVICE_REGENERATE): + return + + async def _regenerate(_call: Any) -> None: + database = _find_live_database(hass) + if database is None: + _LOGGER.warning( + "NestQuest regenerate requested but no config entry has a " + "live database; ignoring" + ) + return + await _run_horizon_materialization(hass, database) + + hass.services.async_register(DOMAIN, SERVICE_REGENERATE, _regenerate) + + +def _deregister_regenerate_service(hass: HomeAssistant) -> None: + """Remove ``nestquest.regenerate`` when the last entry unloads.""" + if hass.services.has_service(DOMAIN, SERVICE_REGENERATE): + hass.services.async_remove(DOMAIN, SERVICE_REGENERATE) + + async def async_setup(hass: HomeAssistant, config: dict[str, Any]) -> bool: """Set up the NestQuest integration. YAML configuration is not used, returns True.""" return True @@ -359,6 +424,8 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: await listener_hass.config_entries.async_reload(listener_entry.entry_id) database = await _async_open_database(hass, await async_get_db_path(hass)) + remove_update_listener: Callable[[], Any] | None = None + remove_time_change_listener: Callable[[], Any] | None = None try: # Seed the admin allowlist on first setup so the owner is never # locked out: an empty database takes the persisted admin copy @@ -387,9 +454,34 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: remove_time_change_listener = _register_day_rollover_listener( hass, database, dict(entry.options) ) + # Backfill any days the daily listener missed while the integration + # was off (or freshly installed): run the one idempotent walk over + # [today, today + DEFAULT_HORIZON_DAYS] once at setup. This runs + # through the SAME path as the daily listener and regenerate service. + await _run_horizon_materialization(hass, database) + _register_regenerate_service(hass) except BaseException: + # A failure anywhere after the database is open must not leak the + # listeners already registered against that (now-closed) database: + # unwind EVERY remover we acquired before the failure, keeping the + # original error as the one raised. Removal is best-effort — a + # failing remover must not mask the setup error that triggered it. + for remove_listener in ( + remove_time_change_listener, + remove_update_listener, + ): + if remove_listener is None: + continue + try: + result = remove_listener() + if inspect.isawaitable(result): + await result + except BaseException: + pass await database.close() raise + assert remove_update_listener is not None + assert remove_time_change_listener is not None runtime_data = NestQuestRuntimeData( entry_id=entry.entry_id, options=dict(entry.options), @@ -406,16 +498,17 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: """Unload a config entry.""" domain_data = hass.data.setdefault(DOMAIN, {}) runtime_data = domain_data.pop(entry.entry_id, None) - if not domain_data: + is_last = not domain_data + if is_last: hass.data.pop(DOMAIN, None) if runtime_data is None: runtime_data = getattr(entry, "runtime_data", None) + unload_error: BaseException | None = None if runtime_data is not None: remove_update_listener = getattr(runtime_data, "remove_update_listener", None) remove_time_change_listener = getattr( runtime_data, "remove_time_change_listener", None ) - unload_error: BaseException | None = None try: for remove_listener in ( remove_update_listener, @@ -439,10 +532,13 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: database = getattr(runtime_data, "database", None) if database is not None: await database.close() - if unload_error is not None: - if getattr(entry, "runtime_data", None) is not None: - entry.runtime_data = None - raise unload_error + # The ``regenerate`` service is DOMAIN-global: it is torn down only + # once the FINAL entry unloads (``hass.data[DOMAIN]`` is now empty), + # never when a sibling entry is still loaded. + if is_last: + _deregister_regenerate_service(hass) if getattr(entry, "runtime_data", None) is not None: entry.runtime_data = None + if unload_error is not None: + raise unload_error return True diff --git a/custom_components/nestquest/const.py b/custom_components/nestquest/const.py index 8167e53..ce21568 100644 --- a/custom_components/nestquest/const.py +++ b/custom_components/nestquest/const.py @@ -9,6 +9,9 @@ LOGGER = logging.getLogger(LOGGER_NAME) PLATFORMS: list[str] = [] +#: Service name for the on-demand regeneration service (``nestquest.regenerate``). +SERVICE_REGENERATE = "regenerate" + SQLITE_DB_FILENAME = "nestquest.db" # Configuration keys and default values diff --git a/custom_components/nestquest/dao_instances.py b/custom_components/nestquest/dao_instances.py index c948aa8..94f05b9 100644 --- a/custom_components/nestquest/dao_instances.py +++ b/custom_components/nestquest/dao_instances.py @@ -59,6 +59,34 @@ def _today() -> datetime.date: return datetime.date.today() +def _resolve_today(today: datetime.date | None) -> datetime.date: + """Return the caller-resolved "today", or the host date when absent. + + The no-past guard and the delete cutoffs must compare due dates + against the SAME calendar date the materialization horizon was built + from. Home Assistant's configured time zone can lag the host clock + around midnight, so when the caller has already resolved the local + date (``today``) that anchor is authoritative; ``None`` falls back to + the host clock (:func:`_today`), which is exactly the previous + behavior for callers with no HA-local date. + """ + return _today() if today is None else today + + +def _reject_past(due_date: str, today_iso: str) -> None: + """Raise unless ``due_date`` is on or after ``today_iso``. + + The single no-past rule shared by :meth:`QuestInstancesDao.upsert` + and :meth:`QuestInstancesDao.upsert_if_valid`: instances are never + generated in the past, measured against the resolved "today". + """ + if due_date < today_iso: + raise ValueError( + f"instances are never generated in the past: due_date " + f"{due_date!r} is before today {today_iso!r}" + ) + + def _validate_utc_timestamp(value: str, field: str) -> None: """Raise ValueError unless ``value`` is a strict UTC ISO-8601 stamp. @@ -178,6 +206,7 @@ class QuestInstancesDao: *, window: str, due_time: str | None = None, + today: datetime.date | None = None, ) -> QuestInstanceRecord: """Create the dated instance for the widened key idempotently. @@ -196,6 +225,10 @@ class QuestInstancesDao: - ``due_date`` must be TODAY or later: instances are never generated in the past (Feature 07). The caller's materialization horizon is future-dated by definition. + ``today`` optionally pins the resolved HA-local date the + guard compares against (falling back to the host clock when + omitted) so a household time zone behind the host around + midnight is not misread as "past". - ``window`` must be one of ``const.QUEST_WINDOWS``. - An existing instance WITH a completion event is IMMUTABLE: the conflict path must not rewrite its snapshot columns @@ -215,24 +248,20 @@ 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 = _today().isoformat() - if due_date < today: - raise ValueError( - f"instances are never generated in the past: due_date " - f"{due_date!r} is before today {today!r}" - ) + # A caller-supplied ``today`` (HA-local) is used for BOTH + # checks and never re-read, since it is already the anchor the + # batch runs against. + today_date = _resolve_today(today) + _reject_past(due_date, today_date.isoformat()) async with _connection_lock(self._database): async with self._database.transaction(): # 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 = _today().isoformat() - if due_date < today: - raise ValueError( - f"instances are never generated in the past: " - f"due_date {due_date!r} is before today " - f"{today!r}" - ) + # holds across midnight rollovers. A pinned HA-local + # anchor is exempt from the re-read (see above). + if today is None: + today_date = _today() + _reject_past(due_date, today_date.isoformat()) definition = await self._database.fetch_one( "SELECT 1 FROM quest_definitions WHERE id = ?", (definition_id,), @@ -298,6 +327,7 @@ class QuestInstancesDao: *, window: str, due_time: str | None = None, + today: datetime.date | None = None, ) -> QuestInstanceRecord | None: """Insert one dated instance iff its tuple is still materializable. @@ -327,7 +357,8 @@ class QuestInstancesDao: 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 - preserved from :meth:`upsert` and still raises ValueError. An + preserved from :meth:`upsert` and still raises ValueError; ``today`` + optionally pins the HA-local anchor it compares against. 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 @@ -338,20 +369,13 @@ 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 = _today().isoformat() - if due_date < today: - raise ValueError( - f"instances are never generated in the past: due_date " - f"{due_date!r} is before today {today!r}" - ) + today_date = _resolve_today(today) + _reject_past(due_date, today_date.isoformat()) async with _connection_lock(self._database): async with self._database.transaction(): - today = _today().isoformat() - if due_date < today: - raise ValueError( - f"instances are never generated in the past: " - f"due_date {due_date!r} is before today {today!r}" - ) + if today is None: + today_date = _today() + _reject_past(due_date, today_date.isoformat()) still_valid = await self._database.fetch_one( "SELECT 1 FROM quest_definitions d " "JOIN quest_definition_assignees a " @@ -463,7 +487,7 @@ class QuestInstancesDao: return [_instance_from_row(row) for row in rows] async def delete_future_uncompleted( - self, definition_id: int, cutoff_date: str + self, definition_id: int, cutoff_date: str, *, today: datetime.date | None = None ) -> int: """Delete the definition's open instances at/after cutoff. @@ -475,9 +499,11 @@ class QuestInstancesDao: is measured against TODAY (materialization regenerates the rolling horizon from now), so a caller cannot use this to rewrite past open instances either — the day-rollover missed - sweep (Feature 11) handles past-due instances instead. Any - instance with a completion event is never touched, regardless - of its date. Returns the number deleted. + sweep (Feature 11) handles past-due instances instead. ``today`` + optionally pins the HA-local anchor the clamp compares against + (falling back to the host clock when omitted). Any instance with + a completion event is never touched, regardless of its date. + Returns the number deleted. """ _validate_date(cutoff_date, "cutoff_date") async with _connection_lock(self._database): @@ -487,8 +513,8 @@ class QuestInstancesDao: # midnight cannot delete instances that became past # while it waited (backdating is clamped to the # execution date, not the call date). - today = _today().isoformat() - effective_cutoff = max(cutoff_date, today) + today_date = _resolve_today(today) + effective_cutoff = max(cutoff_date, today_date.isoformat()) result = await self._database.execute( "DELETE FROM quest_instances WHERE definition_id = ? " "AND due_date >= ? AND NOT EXISTS (" @@ -500,7 +526,7 @@ class QuestInstancesDao: return result.rowcount async def delete_future_uncompleted_for_child( - self, child_id: int, cutoff_date: str + self, child_id: int, cutoff_date: str, *, today: datetime.date | None = None ) -> tuple[int, str]: """Delete the child's open instances at/after cutoff, across every definition, returning ``(deleted, effective_cutoff)``. @@ -511,8 +537,10 @@ class QuestInstancesDao: one transaction — an instance that gains a completion event 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. + docstring). ``today`` optionally pins the HA-local anchor the clamp + compares against (falling back to the host clock when omitted). + Any instance with a completion event is never touched, regardless + of its date. Returns the number deleted and the effective cutoff that was actually applied (``max(cutoff_date, today)``, read under the @@ -530,8 +558,8 @@ 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 = _today().isoformat() - effective_cutoff = max(cutoff_date, today) + today_date = _resolve_today(today) + effective_cutoff = max(cutoff_date, today_date.isoformat()) result = await self._database.execute( "DELETE FROM quest_instances WHERE child_id = ? " "AND due_date >= ? AND NOT EXISTS (" diff --git a/custom_components/nestquest/materialize.py b/custom_components/nestquest/materialize.py index 342e626..3f22e07 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, _today +from .dao_instances import QuestInstancesDao, _resolve_today from .dao_rules import ( _validate_date, load_materialization_input, @@ -127,6 +127,7 @@ async def materialize( end_date: str, *, child_ids: list[int] | None = None, + today: datetime.date | None = None, ) -> int: """Generate quest instances for the closed range [start_date, end_date]. @@ -142,6 +143,14 @@ async def materialize( after a presence change) cannot touch any unrelated child's rows. When omitted (or None) the walk covers every assignee as before. + ``today`` optionally pins the resolved HA-local date the walk's + no-past guard compares against; it is threaded into every + :meth:`~.dao_instances.QuestInstancesDao.upsert_if_valid` write. When + omitted the host clock (``datetime.date.today``) is used, which is the + historical default. A caller that computed the horizon in Home + Assistant's configured time zone MUST pass that same date here so a + time zone behind the host around midnight is not rejected as "past". + All inputs are read in ONE locked transaction; each tuple's write then goes through the atomic :meth:`~.dao_instances.QuestInstancesDao.upsert_if_valid`, which re-validates the tuple and its current ``due_time`` in the same @@ -195,6 +204,7 @@ async def materialize( generated_at, window=window.window, due_time=window.due_time, + today=today, ) if written is not None: count += 1 @@ -205,6 +215,8 @@ async def materialize( async def regenerate_for_definition( database: NestQuestDatabase, definition_id: int, + *, + today: datetime.date | None = None, ) -> int: """Regenerate a definition's future instances after a config change. @@ -221,17 +233,24 @@ async def regenerate_for_definition( :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 + range. ``today`` optionally pins a caller-resolved HA-local date + (threaded into both the delete cutoff and the re-materialization) + so a household time zone behind the host around midnight stays + consistent. Returns the number of instances the re-materialization upserted across the whole snapshot (idempotent — other active definitions' tuples refresh in place, never duplicate). """ - today = _today() - start_date = today.isoformat() - end_date = (today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS)).isoformat() + today_date = _resolve_today(today) + start_date = today_date.isoformat() + end_date = ( + today_date + 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) + await instances.delete_future_uncompleted( + definition_id, start_date, today=today + ) + return await materialize(database, start_date, end_date, today=today) # HOOK (Feature 09 presence services): ``regenerate_for_child`` is the @@ -250,6 +269,8 @@ async def regenerate_for_definition( async def regenerate_for_child( database: NestQuestDatabase, child_id: int, + *, + today: datetime.date | None = None, ) -> int: """Regenerate a child's future instances after a presence change. @@ -277,13 +298,16 @@ async def regenerate_for_child( :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 for - the child across the whole snapshot. + ``today`` optionally pins a caller-resolved HA-local date, threaded + into the delete cutoff and the re-materialization for the same + host-vs-HA time-zone consistency. Returns the number of instances + the re-materialization upserted for the child across the whole + snapshot. """ if isinstance(child_id, bool) or not isinstance(child_id, int): raise ValueError(f"child_id must be an integer, got {child_id!r}") - today = _today() + today_date = _resolve_today(today) instances = QuestInstancesDao(database) # Use the delete's returned effective cutoff as the materialize anchor, @@ -293,10 +317,10 @@ async def regenerate_for_child( # 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() + child_id, today_date.isoformat(), today=today ) start = datetime.date.fromisoformat(effective_cutoff) end_date = (start + datetime.timedelta(days=DEFAULT_HORIZON_DAYS)).isoformat() return await materialize( - database, start.isoformat(), end_date, child_ids=[child_id] + database, start.isoformat(), end_date, child_ids=[child_id], today=today ) diff --git a/custom_components/nestquest/services.yaml b/custom_components/nestquest/services.yaml new file mode 100644 index 0000000..eeae053 --- /dev/null +++ b/custom_components/nestquest/services.yaml @@ -0,0 +1,12 @@ +# Service declarations for the NestQuest integration (Home Assistant +# services.yaml metadata). Services are domain-scoped: nested under the +# "nestquest" domain, not repeated per config entry. + +regenerate: + name: Regenerate quest instances + description: >- + Regenerate every future quest instance for the household over the rolling + horizon, using the same idempotent materialization as the daily rollover + and the startup backfill. Safe to call at any time: a re-run refreshes the + same rows instead of duplicating them, and completed instances are never + touched. \ No newline at end of file diff --git a/tests/conftest.py b/tests/conftest.py index ff8c5d6..b6603e8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -307,6 +307,50 @@ def _async_track_time_change(hass, action, hour=None, minute=None, second=None): _ha_mock("homeassistant.helpers.event").async_track_time_change = _async_track_time_change +class ServiceRegistry: + """Models HA's ``hass.services`` service registry. + + ``async_register`` stores a handler under its (domain, service) key, + ``has_service`` reports registration, and ``async_remove`` removes it + — exactly the surface Home Assistant's ``ServiceRegistry`` exposes + (there is deliberately NO ``async_unregister`` alias here, so a caller + that uses the wrong method name fails instead of being silently + papered over). ``call`` invokes a stored handler the way HA's service + bus would — awaiting a coroutine result — so tests can exercise a + registered service end-to-end. + """ + + def __init__(self, hass=None): + self._hass = hass + self._services: dict[tuple[str, str], object] = {} + + def async_register(self, domain, service, func, schema=None, supports_response=None): + self._services[(domain, service)] = func + return None + + def has_service(self, domain, service): + return (domain, service) in self._services + + def async_remove(self, domain, service): + self._services.pop((domain, service), None) + return None + + async def call(self, domain, service, data=None): + func = self._services.get((domain, service)) + if func is None: + raise KeyError(f"service {domain}.{service} not registered") + call = SimpleNamespace(domain=domain, service=service, data=data or {}) + result = func(call) + if inspect.isawaitable(result): + await result + return result + + @property + def registered_services(self): + """The registered (domain, service) pairs.""" + return list(self._services) + + def make_config_entry( entry_id: str = "test_entry", options: dict | None = None, @@ -369,6 +413,7 @@ def make_hass() -> tuple: hass.async_add_executor_job = _run_executor_job registry = ListenerRegistry(hass) hass.time_change = TimeChangeRegistry(hass) + hass.services = ServiceRegistry(hass) # A valid IANA time zone (the day-rollover listener reads it to # compute "today" in HA local time). hass.config.time_zone = "UTC" diff --git a/tests/test_lifecycle.py b/tests/test_lifecycle.py index f56864f..68be6ce 100644 --- a/tests/test_lifecycle.py +++ b/tests/test_lifecycle.py @@ -11,6 +11,7 @@ from custom_components.nestquest import DOMAIN, async_setup_entry, async_unload_ from custom_components.nestquest.const import ( CONF_DAY_ROLLOVER_TIME, DEFAULT_HORIZON_DAYS, + SERVICE_REGENERATE, DOMAIN as DOMAIN_CONST, ) @@ -345,7 +346,9 @@ async def test_day_rollover_listener_materializes_ha_local_horizon( await hass.time_change.fire() - fake.assert_awaited_once_with(database, expected_start, expected_end) + fake.assert_awaited_once_with( + database, expected_start, expected_end, today=today + ) def test_day_rollover_time_change_tracker_rejects_local_kwarg() -> None: @@ -381,4 +384,173 @@ async def test_setup_applies_all_v1_tables(hass, make_entry) -> None: "quest_definitions", "schedule_rules", ] - await async_unload_entry(hass, entry) \ No newline at end of file + await async_unload_entry(hass, entry) + + +async def _seed_daily_child_and_definition(database, today) -> int: + """Create one always-present child + a daily definition starting today. + + Returns the child's id. No presence schedule means the child is present + every day, so the daily rule yields exactly one instance per horizon day. + """ + from custom_components.nestquest.dao_children import ChildrenDao + from custom_components.nestquest.quest_definitions import ( + create_quest_definition, + ) + from custom_components.nestquest.recurrence import RuleType, ScheduleRule + + NOW = "2026-09-14T12:00:00+00:00" + child = await ChildrenDao(database).create("Ada", NOW) + await create_quest_definition( + database, + "Daily chore", + ScheduleRule(rule_type=RuleType.DAILY, start_date=today.isoformat()), + [child.id], + ["morning"], + ) + return child.id + + +def _local_today(hass) -> "datetime.date": + import datetime + from zoneinfo import ZoneInfo + + return datetime.datetime.now(ZoneInfo(hass.config.time_zone)).date() + + +async def test_setup_backfills_instances_over_horizon(hass, make_entry) -> None: + """Setup materializes once so a restart backfills any missed days.""" + import datetime + + from custom_components.nestquest.dao_instances import QuestInstancesDao + from custom_components.nestquest.db import NestQuestDatabase + from custom_components.nestquest.migrations import apply_migrations + from custom_components.nestquest.store import async_get_db_path + + # Seed the database BEFORE setup: a restart reopens an existing file that + # already holds definitions. The startup backfill must pick them up. + db_path = await async_get_db_path(hass) + pre = NestQuestDatabase(hass) + await pre.open(db_path) + await apply_migrations(pre) + child_id = await _seed_daily_child_and_definition(pre, _local_today(hass)) + await pre.close() + + entry = _wire(make_entry(), hass.registry) + assert await async_setup_entry(hass, entry) is True + + today = _local_today(hass) + end = (today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS)).isoformat() + records = await QuestInstancesDao(entry.runtime_data.database).list_by_date_range( + child_id, today.isoformat(), end + ) + assert len(records) == DEFAULT_HORIZON_DAYS + 1 + + +async def test_regenerate_service_registered_and_materializes( + hass, make_entry +) -> None: + """``nestquest.regenerate`` is registered and calling it regenerates.""" + import datetime + + from custom_components.nestquest.dao_instances import QuestInstancesDao + + entry = _wire(make_entry(), hass.registry) + assert await async_setup_entry(hass, entry) is True + assert hass.services.has_service(DOMAIN, SERVICE_REGENERATE) + + # A definition created AFTER setup has no instances yet (create does not + # materialize); the service must generate them on demand. + database = entry.runtime_data.database + child_id = await _seed_daily_child_and_definition(database, _local_today(hass)) + + await hass.services.call(DOMAIN, SERVICE_REGENERATE) + + today = _local_today(hass) + end = (today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS)).isoformat() + records = await QuestInstancesDao(database).list_by_date_range( + child_id, today.isoformat(), end + ) + assert len(records) == DEFAULT_HORIZON_DAYS + 1 + + +async def test_unload_deregisters_regenerate_service(hass, make_entry) -> None: + """Unload removes the ``regenerate`` service so no stale entry leaks.""" + entry = _wire(make_entry(), hass.registry) + assert await async_setup_entry(hass, entry) is True + assert hass.services.has_service(DOMAIN, SERVICE_REGENERATE) + + assert await async_unload_entry(hass, entry) is True + assert not hass.services.has_service(DOMAIN, SERVICE_REGENERATE) + + +async def test_regenerate_service_is_domain_scoped_across_entries( + hass, make_entry +) -> None: + """The ``regenerate`` service is domain-global, not per-entry. + + With two loaded entries the service must be registered and functional; + unloading one entry must keep it registered and functional for the + remaining entry; unloading the final entry must remove it. + """ + import datetime + + from custom_components.nestquest.dao_instances import QuestInstancesDao + + entry_a = _wire(make_entry(entry_id="entry_a"), hass.registry) + entry_b = _wire(make_entry(entry_id="entry_b"), hass.registry) + + assert await async_setup_entry(hass, entry_a) is True + assert await async_setup_entry(hass, entry_b) is True + assert hass.services.has_service(DOMAIN, SERVICE_REGENERATE) + + # The domain-global handler resolves a live database from any entry. + database_b = entry_b.runtime_data.database + child_id = await _seed_daily_child_and_definition( + database_b, _local_today(hass) + ) + await hass.services.call(DOMAIN, SERVICE_REGENERATE) + today = _local_today(hass) + end = (today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS)).isoformat() + records = await QuestInstancesDao(database_b).list_by_date_range( + child_id, today.isoformat(), end + ) + assert len(records) == DEFAULT_HORIZON_DAYS + 1 + + # Unloading one entry must not tear down the shared service. + assert await async_unload_entry(hass, entry_a) is True + assert hass.services.has_service(DOMAIN, SERVICE_REGENERATE) + + # It still works for the remaining entry. + await hass.services.call(DOMAIN, SERVICE_REGENERATE) + + # Unloading the final entry removes the domain-global service. + assert await async_unload_entry(hass, entry_b) is True + assert not hass.services.has_service(DOMAIN, SERVICE_REGENERATE) + + +async def test_setup_cleans_listeners_when_materialization_raises( + hass, make_entry, monkeypatch +) -> None: + """A startup-materialization failure rolls back every registered listener. + + The daily and update listeners are installed before the startup + materialization runs; when that walk raises, the database is closed but + the remover callables must also be invoked so no listener survives + firing against a closed database. + """ + import custom_components.nestquest as nestquest + + async def _boom(*_args, **_kwargs): + raise RuntimeError("materialization failed") + + monkeypatch.setattr(nestquest, "_materialize_run", _boom) + + entry = _wire(make_entry(), hass.registry) + with pytest.raises(RuntimeError, match="materialization failed"): + await async_setup_entry(hass, entry) + + assert hass.time_change.size == 0 + assert hass.registry.size == 0 + assert not hass.services.has_service(DOMAIN, SERVICE_REGENERATE) + assert not hasattr(entry, "runtime_data") \ No newline at end of file diff --git a/tests/test_materialize.py b/tests/test_materialize.py index 62523fd..c20f226 100644 --- a/tests/test_materialize.py +++ b/tests/test_materialize.py @@ -340,7 +340,7 @@ async def _materialize_with_change( seen = {"n": 0} async def _gated(self, definition_id, child_id, due_date, generated_at, - *, window, due_time=None): + *, window, due_time=None, today=None): seen["n"] += 1 if seen["n"] == 1: seen["first"] = (definition_id, child_id, window) @@ -348,7 +348,7 @@ async def _materialize_with_change( await release.wait() return await original_upsert( self, definition_id, child_id, due_date, generated_at, - window=window, due_time=due_time, + window=window, due_time=due_time, today=today, ) QuestInstancesDao.upsert_if_valid = _gated @@ -630,6 +630,42 @@ def test_materialize_rejects_malformed_or_inverted_bounds(tmp_path) -> None: _with_db(tmp_path, "materialize-bounds.db")(_body) +def test_materialize_accepts_ha_local_today_behind_host(tmp_path) -> None: + """The no-past guard compares against a caller-supplied HA-local today. + + A household time zone behind the host clock around midnight resolves a + HA-local "today" one calendar day earlier than the host's + ``date.today()``. The host-clock guard would reject that date as past, + so the resolved HA-local date must be threaded through ``materialize`` + and accepted instead of raising. + """ + async def _body(database): + children = ChildrenDao(database) + child = await children.create("Ada", NOW) + ha_today = datetime.date.today() - datetime.timedelta(days=1) + start_iso = ha_today.isoformat() + end_iso = (ha_today + datetime.timedelta(days=14)).isoformat() + await create_quest_definition( + database, + "Daily chore", + ScheduleRule(rule_type=RuleType.DAILY, start_date=start_iso), + [child.id], + ["morning"], + ) + + # Without the pinned HA-local today, the host-clock guard rejects + # the horizon start (ha_today is "yesterday" on the host). + with pytest.raises(ValueError, match="never generated in the past"): + await materialize(database, start_iso, end_iso) + + # With today=ha_today, the same range is accepted and materialized. + count = await materialize(database, start_iso, end_iso, today=ha_today) + assert count == 15 + return count + + _with_db(tmp_path, "materialize-ha-local-today.db")(_body) + + def test_materialize_skips_inactive_definitions(tmp_path) -> None: async def _body(database): from custom_components.nestquest.quest_definitions import ( @@ -914,10 +950,8 @@ def test_regenerate_for_child_uses_single_anchor_across_midnight( 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) diff --git a/tests/test_services.py b/tests/test_services.py new file mode 100644 index 0000000..68f2cde --- /dev/null +++ b/tests/test_services.py @@ -0,0 +1,41 @@ +"""Tests for the NestQuest services.yaml declaration file.""" +from __future__ import annotations + +import re +from pathlib import Path + +SERVICES_PATH = ( + Path(__file__).parent.parent + / "custom_components" + / "nestquest" + / "services.yaml" +) + + +def test_services_yaml_exists() -> None: + assert SERVICES_PATH.exists(), f"services.yaml missing at {SERVICES_PATH}" + + +def test_services_yaml_declares_regenerate() -> None: + text = SERVICES_PATH.read_text(encoding="utf-8") + assert re.search(r"^regenerate:\s*$", text, re.MULTILINE), ( + "services.yaml must declare the 'regenerate' service" + ) + + +def test_services_yaml_regenerate_has_description_and_no_fields() -> None: + """The regenerate service is documented with a clear description and no + required fields (there is no ``fields`` block under it).""" + text = SERVICES_PATH.read_text(encoding="utf-8") + match = re.search(r"^regenerate:\s*$", text, re.MULTILINE) + assert match is not None + section = text[match.end():] + next_service = re.search(r"^(?![#\s])\w+:\s*$", section, re.MULTILINE) + if next_service is not None: + section = section[: next_service.start()] + assert re.search(r"description:", section), ( + "regenerate must carry a description" + ) + assert "fields:" not in section, ( + "regenerate declares no required fields" + ) \ No newline at end of file From f6aad9d20f840bc6d20facabae76af495b1ff651 Mon Sep 17 00:00:00 2001 From: Joshua Date: Thu, 17 Sep 2026 16:06:41 -0400 Subject: [PATCH 08/12] HA-local timezone + DST boundary tests (Feature 07, task 8) (#82) --- .../nestquest/quest_definitions.py | 32 +- tests/test_timezones.py | 370 ++++++++++++++++++ 2 files changed, 398 insertions(+), 4 deletions(-) create mode 100644 tests/test_timezones.py diff --git a/custom_components/nestquest/quest_definitions.py b/custom_components/nestquest/quest_definitions.py index 817f7fc..86ada89 100644 --- a/custom_components/nestquest/quest_definitions.py +++ b/custom_components/nestquest/quest_definitions.py @@ -271,6 +271,7 @@ async def edit_quest_definition( icon: str | None | object = _UNSET, rule: ScheduleRule | object = _UNSET, windows: list[str | tuple[str, str | None]] | object = _UNSET, + today: datetime.date | None = None, ) -> CreatedQuestDefinition: """Edit a quest definition's metadata, rule and windows atomically. @@ -292,6 +293,13 @@ async def edit_quest_definition( so the returned bundle is a consistent edit snapshot. Edits change future instances only; no ``quest_instances`` or ``completion_events`` row is touched. + + ``today`` optionally pins the caller-resolved HA-local date the + regeneration's no-past guard and horizon are anchored to (threaded + into :func:`~.materialize.regenerate_for_definition`); it is a plain + ``datetime.date`` with no HA import here — the Feature 09 service + supplies the hass-derived value. When omitted, regeneration falls + back to the host clock, same as before. """ _validate_definition_id(definition_id) @@ -342,7 +350,7 @@ async def edit_quest_definition( schedule_rule_storage_from_record(snapshot.rule) ) - await regenerate_for_definition(database, definition_id) + await regenerate_for_definition(database, definition_id, today=today) return CreatedQuestDefinition( definition=snapshot.definition, @@ -356,6 +364,8 @@ async def assign_child( database: NestQuestDatabase, definition_id: int, child_id: int, + *, + today: datetime.date | None = None, ) -> list[ChildRecord]: """Assign ``child_id`` to the definition (idempotent). @@ -369,6 +379,9 @@ async def assign_child( write). Raises ValueError on any rejected argument. Only the ``quest_definition_assignees`` link is written; existing instances and completion history are never touched. + + ``today`` optionally pins the caller-resolved HA-local date threaded + into the assignment's regeneration (see :func:`edit_quest_definition`). """ _validate_definition_id(definition_id) _validate_child_id(child_id) @@ -384,7 +397,7 @@ 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) + await regenerate_for_definition(database, definition_id, today=today) return assignees @@ -392,6 +405,8 @@ async def unassign_child( database: NestQuestDatabase, definition_id: int, child_id: int, + *, + today: datetime.date | None = None, ) -> bool: """Unassign ``child_id`` from the definition; True when removed. @@ -401,6 +416,9 @@ async def unassign_child( was not currently assigned. Raises ValueError on a rejected argument. Only the ``quest_definition_assignees`` link is written; existing instances and completion history are never touched. + + ``today`` optionally pins the caller-resolved HA-local date threaded + into the unassignment's regeneration (see :func:`edit_quest_definition`). """ _validate_definition_id(definition_id) _validate_child_id(child_id) @@ -411,7 +429,7 @@ async def unassign_child( f"definition_id: quest definition {definition_id} does not exist" ) removed = await dao.remove_assignee(definition_id, child_id) - await regenerate_for_definition(database, definition_id) + await regenerate_for_definition(database, definition_id, today=today) return removed @@ -419,6 +437,8 @@ async def set_quest_definition_active( database: NestQuestDatabase, definition_id: int, is_active: bool, + *, + today: datetime.date | None = None, ) -> CreatedQuestDefinition: """Deactivate or reactivate a definition; returns the updated view. @@ -441,6 +461,10 @@ async def set_quest_definition_active( definition, its rule, assignees and windows all run inside the connection-scoped lock, so the returned bundle is a consistent snapshot that a concurrent mutation cannot race. + + ``today`` optionally pins the caller-resolved HA-local date threaded + into the activation change's regeneration (see + :func:`edit_quest_definition`). """ _validate_definition_id(definition_id) if not isinstance(is_active, bool): @@ -464,7 +488,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) + await regenerate_for_definition(database, definition_id, today=today) return CreatedQuestDefinition( definition=updated, rule=rule, diff --git a/tests/test_timezones.py b/tests/test_timezones.py new file mode 100644 index 0000000..273b588 --- /dev/null +++ b/tests/test_timezones.py @@ -0,0 +1,370 @@ +"""Timezone tests for the single HA-local date source in the generation path. + +These pin the done-condition that every "today" read in the generation path +derives from ``hass.config.time_zone`` — not the host clock and not UTC — +and that the rolling horizon ``[today, today + DEFAULT_HORIZON_DAYS]`` and the +no-past guard resolve the correct local dates around midnight and across the +daylight-saving transitions. +""" +from __future__ import annotations + +import asyncio +import datetime +from types import SimpleNamespace +from zoneinfo import ZoneInfo + +from custom_components.nestquest.const import DEFAULT_HORIZON_DAYS +from custom_components.nestquest.dao_children import ChildrenDao +from custom_components.nestquest.dao_instances import QuestInstancesDao +from custom_components.nestquest.db import NestQuestDatabase +from custom_components.nestquest.migrations import apply_migrations +from custom_components.nestquest.quest_definitions import ( + assign_child, + create_quest_definition, + edit_quest_definition, +) +from custom_components.nestquest.recurrence import RuleType, ScheduleRule + +NOW = "2026-09-14T12:00:00+00:00" +NEW_YORK = "America/New_York" + + +def _make_hass_mock(): + from unittest.mock import AsyncMock, MagicMock + + hass = MagicMock() + hass.async_add_executor_job = AsyncMock(side_effect=(lambda fn, *a: fn(*a))) + return hass + + +async def _prepare(path): + database = NestQuestDatabase(_make_hass_mock()) + await database.open(path) + await apply_migrations(database) + return database + + +def _run(coro): + return asyncio.new_event_loop().run_until_complete(coro) + + +def _freeze_clock( + monkeypatch, instant: datetime.datetime +) -> tuple[datetime.datetime, list]: + """Freeze the integration's ``datetime`` clock to a fixed aware instant. + + Only ``custom_components.nestquest``'s ``datetime`` binding is patched, so + the generation path's "today" reads this instant (localized to whatever + time zone it asks for) while every other module and the test harness keep + the real clock. Returns ``(frozen, requested)`` where ``frozen`` is the + instant normalized to UTC and ``requested`` is the list of ``tzinfo`` + objects the code under test passed to ``now`` — so a test can prove the + generation path actually requested the configured HA time zone rather + than silently reading UTC. + """ + import custom_components.nestquest as nestquest + + frozen = instant.astimezone(datetime.timezone.utc) + requested: list = [] + + class _FrozenDatetime(datetime.datetime): + @classmethod + def now(cls, tz=None): + requested.append(tz) + if tz is None: + return frozen.replace(tzinfo=None) + return frozen.astimezone(tz) + + fake_module = SimpleNamespace( + datetime=_FrozenDatetime, + timedelta=datetime.timedelta, + date=datetime.date, + ) + monkeypatch.setattr(nestquest, "datetime", fake_module) + return frozen, requested + + +def _assert_requested_ha_timezone(requested, time_zone: str) -> None: + """Assert ``now`` was called with (only) the configured HA time zone. + + This is what turns the DST tests into a real proof of the HA-local date + source: if the generation path regressed to ``now(timezone.utc)``, its + requested tz would be UTC and this fails. + """ + assert requested, "the generation path never called now(tz)" + assert all(tz == ZoneInfo(time_zone) for tz in requested) + + +def _tz_hass(time_zone: str): + """A minimal hass exposing only the ``config.time_zone`` the walk reads.""" + return SimpleNamespace(config=SimpleNamespace(time_zone=time_zone)) + + +async def _seed_daily(database, start_iso: str) -> int: + """One always-present child + a daily definition starting ``start_iso``.""" + child = await ChildrenDao(database).create("Ada", NOW) + await create_quest_definition( + database, + "Daily chore", + ScheduleRule(rule_type=RuleType.DAILY, start_date=start_iso), + [child.id], + ["morning"], + ) + return child.id + + +def _local_date(instant_utc: datetime.datetime, time_zone: str) -> datetime.date: + return instant_utc.astimezone(ZoneInfo(time_zone)).date() + + +def _assert_contiguous_horizon(records, local_today: datetime.date) -> None: + """The horizon is exactly today..today+horizon, no off-by-one, no gap.""" + expected = { + (local_today + datetime.timedelta(days=offset)).isoformat() + for offset in range(DEFAULT_HORIZON_DAYS + 1) + } + assert {r.due_date for r in records} == expected + assert len(records) == DEFAULT_HORIZON_DAYS + 1 + + +def test_horizon_uses_newyork_local_date_after_local_midnight( + tmp_path, monkeypatch +) -> None: + """A run just after New York local midnight uses the local date. + + The frozen instant is 00:05 New York local time on 2026-03-10 (EDT, so + 04:05 UTC). The horizon must start on 2026-03-10 — the local calendar + date — not any host/UTC-derived value. + """ + instant = datetime.datetime( + 2026, 3, 10, 0, 5, tzinfo=ZoneInfo(NEW_YORK) + ) + local_today = instant.date() + + async def _body(database): + hass = _tz_hass(NEW_YORK) + _frozen, requested = _freeze_clock(monkeypatch, instant) + import custom_components.nestquest as nestquest + + child_id = await _seed_daily(database, local_today.isoformat()) + await nestquest._run_horizon_materialization(hass, database) + records = await QuestInstancesDao(database).list_by_date_range( + child_id, + local_today.isoformat(), + (local_today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS)).isoformat(), + ) + _assert_contiguous_horizon(records, local_today) + _assert_requested_ha_timezone(requested, NEW_YORK) + return len(records) + + async def _main(): + database = await _prepare(tmp_path / "tz-midnight.db") + try: + return await _body(database) + finally: + await database.close() + + assert _run(_main()) == DEFAULT_HORIZON_DAYS + 1 + + +def test_horizon_uses_local_date_not_utc_when_host_ahead( + tmp_path, monkeypatch +) -> None: + """The walk's "today" is HA-local, never the UTC/system calendar date. + + FROZEN at 2026-01-15 03:00 UTC: the UTC calendar date is 2026-01-15, but + New York local is still 2026-01-14 22:00 EST. The horizon must start on + 2026-01-14 (local), proving the UTC date did not win. + """ + instant = datetime.datetime(2026, 1, 15, 3, 0, tzinfo=datetime.timezone.utc) + local_today = _local_date(instant, NEW_YORK) + assert local_today == datetime.date(2026, 1, 14) + assert instant.date() == datetime.date(2026, 1, 15) + + async def _body(database): + hass = _tz_hass(NEW_YORK) + _frozen, requested = _freeze_clock(monkeypatch, instant) + import custom_components.nestquest as nestquest + + child_id = await _seed_daily(database, local_today.isoformat()) + await nestquest._run_horizon_materialization(hass, database) + records = await QuestInstancesDao(database).list_by_date_range( + child_id, + local_today.isoformat(), + (local_today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS)).isoformat(), + ) + _assert_contiguous_horizon(records, local_today) + _assert_requested_ha_timezone(requested, NEW_YORK) + return len(records) + + async def _main(): + database = await _prepare(tmp_path / "tz-host-ahead.db") + try: + return await _body(database) + finally: + await database.close() + + assert _run(_main()) == DEFAULT_HORIZON_DAYS + 1 + + +def test_horizon_resolves_spring_forward_day_without_off_by_one( + tmp_path, monkeypatch +) -> None: + """Across the 2026-03-08 spring-forward, today and the horizon stay local. + + Before the jump ("01:30 EST") and after it ("03:30 EDT") both resolve to + 2026-03-08 local — the lost hour must not skip or repeat a calendar day — + and the full horizon remains 15 contiguous days. + """ + zone = ZoneInfo(NEW_YORK) + before = datetime.datetime(2026, 3, 8, 1, 30, tzinfo=zone) # EST + after = datetime.datetime(2026, 3, 8, 3, 30, tzinfo=zone) # EDT + assert before.date() == after.date() == datetime.date(2026, 3, 8) + + async def _body(database): + hass = _tz_hass(NEW_YORK) + import custom_components.nestquest as nestquest + + requested_batches: list[list] = [] + for instant in (before, after): + _frozen, requested = _freeze_clock(monkeypatch, instant) + requested_batches.append(requested) + local_today = instant.date() + child_id = await _seed_daily(database, local_today.isoformat()) + await nestquest._run_horizon_materialization(hass, database) + records = await QuestInstancesDao(database).list_by_date_range( + child_id, + local_today.isoformat(), + (local_today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS)).isoformat(), + ) + _assert_contiguous_horizon(records, local_today) + requested_all = [tz for batch in requested_batches for tz in batch] + _assert_requested_ha_timezone(requested_all, NEW_YORK) + return local_today + + async def _main(): + database = await _prepare(tmp_path / "tz-spring-forward.db") + try: + return await _body(database) + finally: + await database.close() + + assert _run(_main()) == datetime.date(2026, 3, 8) + + +def test_horizon_resolves_fall_back_day_without_off_by_one( + tmp_path, monkeypatch +) -> None: + """Across the 2026-11-01 fall-back, today and the horizon stay local. + + The repeated 01:00-02:00 hour must not resolve to the wrong calendar + day: the first 01:30 (``fold=0``, EDT) and the repeated 01:30 + (``fold=1``, EST) are distinct instants yet both resolve to 2026-11-01 + — and both, along with 03:30 EST after the jump, materialize the same + 15 contiguous-day horizon with no off-by-one. + """ + zone = ZoneInfo(NEW_YORK) + first = datetime.datetime(2026, 11, 1, 1, 30, tzinfo=zone, fold=0) # EDT + second = datetime.datetime(2026, 11, 1, 1, 30, tzinfo=zone, fold=1) # EST + after = datetime.datetime(2026, 11, 1, 3, 30, tzinfo=zone) # EST + assert first.date() == second.date() == after.date() == datetime.date(2026, 11, 1) + assert first.utcoffset() != second.utcoffset() # the two repeated-hour folds + + async def _body(database): + hass = _tz_hass(NEW_YORK) + import custom_components.nestquest as nestquest + + requested_batches: list[list] = [] + for instant in (first, second, after): + _frozen, requested = _freeze_clock(monkeypatch, instant) + requested_batches.append(requested) + local_today = instant.date() + child_id = await _seed_daily(database, local_today.isoformat()) + await nestquest._run_horizon_materialization(hass, database) + records = await QuestInstancesDao(database).list_by_date_range( + child_id, + local_today.isoformat(), + (local_today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS)).isoformat(), + ) + _assert_contiguous_horizon(records, local_today) + requested_all = [tz for batch in requested_batches for tz in batch] + _assert_requested_ha_timezone(requested_all, NEW_YORK) + return local_today + + async def _main(): + database = await _prepare(tmp_path / "tz-fall-back.db") + try: + return await _body(database) + finally: + await database.close() + + assert _run(_main()) == datetime.date(2026, 11, 1) + + +def test_definition_business_regeneration_uses_pinned_today_not_host( + tmp_path, monkeypatch +) -> None: + """``edit_quest_definition``/``assign_child`` anchor regeneration on + the caller-pinned HA-local ``today``, not the host clock. + + The host clock (the ``_today`` fallback) is pinned one day AHEAD of the + HA-local ``today``; the guard would reject the local "today" as past + (and the horizon would skip it) if it consulted the host clock. Both + business operations must regenerate against the pinned date and keep + that first local day, proving the threaded ``today`` wins. + """ + import custom_components.nestquest.dao_instances as dao_instances_module + + pinned = datetime.date(2026, 1, 14) + host_today = datetime.date(2026, 1, 15) + monkeypatch.setattr(dao_instances_module, "_today", lambda: host_today) + + async def _body(database): + children = ChildrenDao(database) + dao = QuestInstancesDao(database) + horizon_end = pinned + datetime.timedelta(days=DEFAULT_HORIZON_DAYS) + + a = await children.create("Ada", NOW) + created = await create_quest_definition( + database, + "Daily chore", + ScheduleRule(rule_type=RuleType.DAILY, start_date=pinned.isoformat()), + [a.id], + [("morning", "09:00")], + ) + definition_id = created.definition.id + + # edit anchors on pinned: the horizon starts at the local day the + # host clock calls "past", so its presence proves pinned won. + await edit_quest_definition( + database, + definition_id, + windows=[("morning", "10:15")], + today=pinned, + ) + a_records = await dao.list_by_date_range( + a.id, pinned.isoformat(), horizon_end.isoformat() + ) + assert [r.due_date for r in a_records][0] == pinned.isoformat() + assert {r.due_time for r in a_records} == {"10:15"} + assert len(a_records) == DEFAULT_HORIZON_DAYS + 1 + + # assign anchors on pinned too: the new child's regeneration keeps + # the same pinned-local first day rather than skipping to the host day. + b = await children.create("Bo", NOW) + await assign_child(database, definition_id, b.id, today=pinned) + b_records = await dao.list_by_date_range( + b.id, pinned.isoformat(), horizon_end.isoformat() + ) + assert [r.due_date for r in b_records][0] == pinned.isoformat() + assert len(b_records) == DEFAULT_HORIZON_DAYS + 1 + return definition_id + + async def _main(): + database = await _prepare(tmp_path / "tz-definition-regen.db") + try: + return await _body(database) + finally: + await database.close() + + assert _run(_main()) is not None From 353d88e3afb48bc5ec494edc07958b4ed3c0d5f1 Mon Sep 17 00:00:00 2001 From: Joshua Date: Thu, 17 Sep 2026 16:12:08 -0400 Subject: [PATCH 09/12] Clamp start to today (no past instances) (Feature 07, task 9) (#83) --- custom_components/nestquest/materialize.py | 15 +++++ tests/test_materialize.py | 64 +++++++++++++++++++--- 2 files changed, 70 insertions(+), 9 deletions(-) diff --git a/custom_components/nestquest/materialize.py b/custom_components/nestquest/materialize.py index 3f22e07..e75c040 100644 --- a/custom_components/nestquest/materialize.py +++ b/custom_components/nestquest/materialize.py @@ -151,6 +151,14 @@ async def materialize( Assistant's configured time zone MUST pass that same date here so a time zone behind the host around midnight is not rejected as "past". + ``start_date`` is clamped up to the resolved "today" (the same + ``today`` anchor threaded into the no-past guard), so a caller that + passes a ``start_date`` already in the past never yields a + past-dated instance: the walk simply begins on today and skips every + earlier date. A range entirely in the past therefore produces zero + instances rather than an error, and the DAO's no-past guard remains + the authoritative backstop under the lock. + All inputs are read in ONE locked transaction; each tuple's write then goes through the atomic :meth:`~.dao_instances.QuestInstancesDao.upsert_if_valid`, which re-validates the tuple and its current ``due_time`` in the same @@ -184,6 +192,13 @@ async def materialize( start = datetime.datetime.strptime(start_date, "%Y-%m-%d").date() end = datetime.datetime.strptime(end_date, "%Y-%m-%d").date() + # Clamp a past start up to today (the HA-local anchor when the caller + # pinned one, else the host clock) so the walk can never emit a + # past-dated instance; the DAO's no-past guard remains the backstop. + # A range entirely in the past therefore yields zero instances. + today_date = _resolve_today(today) + start = max(start, today_date) + count = 0 cursor = start while cursor <= end: diff --git a/tests/test_materialize.py b/tests/test_materialize.py index c20f226..e4b29d8 100644 --- a/tests/test_materialize.py +++ b/tests/test_materialize.py @@ -631,13 +631,14 @@ def test_materialize_rejects_malformed_or_inverted_bounds(tmp_path) -> None: def test_materialize_accepts_ha_local_today_behind_host(tmp_path) -> None: - """The no-past guard compares against a caller-supplied HA-local today. + """The clamp and no-past guard compare against a caller-supplied HA-local today. A household time zone behind the host clock around midnight resolves a HA-local "today" one calendar day earlier than the host's - ``date.today()``. The host-clock guard would reject that date as past, - so the resolved HA-local date must be threaded through ``materialize`` - and accepted instead of raising. + ``date.today()``. The walk clamps its start to that anchor: without + a pinned HA-local date the effective start is the host's today (one + day after ha_today), so the now-past day is skipped rather than + rejected, and only the host-today-onward days materialize. """ async def _body(database): children = ChildrenDao(database) @@ -653,12 +654,13 @@ def test_materialize_accepts_ha_local_today_behind_host(tmp_path) -> None: ["morning"], ) - # Without the pinned HA-local today, the host-clock guard rejects - # the horizon start (ha_today is "yesterday" on the host). - with pytest.raises(ValueError, match="never generated in the past"): - await materialize(database, start_iso, end_iso) + # Without a pinned HA-local today, the walk clamps its start to the + # host's today (one day after ha_today); the past day is dropped and + # the 14 host-today-onward days materialize. + without_pin = await materialize(database, start_iso, end_iso) + assert without_pin == 14 - # With today=ha_today, the same range is accepted and materialized. + # With today=ha_today, the full 15-day horizon materializes. count = await materialize(database, start_iso, end_iso, today=ha_today) assert count == 15 return count @@ -666,6 +668,50 @@ def test_materialize_accepts_ha_local_today_behind_host(tmp_path) -> None: _with_db(tmp_path, "materialize-ha-local-today.db")(_body) +def test_materialize_clamps_past_start_to_today(tmp_path) -> None: + """A past start_date is clamped to today: no past-dated instance is created. + + A caller passing a start one month in the past never yields a + past-dated instance: the walk clamps its effective start up to today + and materializes only dates on/after today, leaving the past untouched + rather than raising or corrupting state. + """ + async def _body(database): + children = ChildrenDao(database) + child = await children.create("Ada", NOW) + + today = datetime.date.today() + start_iso = (today - datetime.timedelta(days=30)).isoformat() + end_iso = (today + datetime.timedelta(days=14)).isoformat() + + await create_quest_definition( + database, + "Daily chore", + ScheduleRule(rule_type=RuleType.DAILY, start_date=start_iso), + [child.id], + ["morning"], + ) + + count = await materialize(database, start_iso, end_iso, today=today) + assert count == 15 + + dao = QuestInstancesDao(database) + records = await dao.list_by_date_range(child.id, start_iso, end_iso) + assert len(records) == 15 + assert all(r.due_date >= today.isoformat() for r in records) + + # The past month is empty: no past-dated instance was ever created. + past_records = await dao.list_by_date_range( + child.id, + start_iso, + (today - datetime.timedelta(days=1)).isoformat(), + ) + assert past_records == [] + return count + + _with_db(tmp_path, "materialize-past-start.db")(_body) + + def test_materialize_skips_inactive_definitions(tmp_path) -> None: async def _body(database): from custom_components.nestquest.quest_definitions import ( From c001aca62d3cd2c169db192abab80dd24ef1974c Mon Sep 17 00:00:00 2001 From: Joshua Date: Thu, 17 Sep 2026 16:38:56 -0400 Subject: [PATCH 10/12] End-to-end 28-day generation (Feature 07, task 10) (#84) --- tests/test_materialize.py | 336 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 336 insertions(+) diff --git a/tests/test_materialize.py b/tests/test_materialize.py index e4b29d8..390b3ea 100644 --- a/tests/test_materialize.py +++ b/tests/test_materialize.py @@ -1113,6 +1113,342 @@ def test_regenerate_for_child_is_child_scoped(tmp_path) -> None: _with_db(tmp_path, "regenerate-child-scoped.db")(_body) +#: Fixed end-to-end anchor: 2026-06-01 is a Monday AND the 1st of its month, +#: so the two-week custody cycle is week-aligned and the yearly rule's +#: "1st of the anchor month" is GUARANTEED to fire inside the 28-day window +#: (pin ``today=anchor`` throughout so the walk never clamps this fixed +#: calendar date up to the host clock). +_E2E_ANCHOR = "2026-06-01" + +#: Hand-checked oracle over 2026-06-01 .. 2026-06-28 (69 entries). Each row +#: is one independent (definition, child, window, due_date, due_time); the +#: definition/child slugs are substituted with runtime ids, but the dates are +#: literal, reviewed by hand — no recurrence/presence math is re-run here. +#: Custody (2-week cycle anchored on the Monday): A present weeks 0 and 2, +#: B present weeks 1 and 3, C present every day. +_E2E_EXPECTED = [ + # daily [A, C] morning 09:00, every day. A present weeks 0, 2. + ("daily", "A", "morning", "2026-06-01", "09:00"), + ("daily", "A", "morning", "2026-06-02", "09:00"), + ("daily", "A", "morning", "2026-06-03", "09:00"), + ("daily", "A", "morning", "2026-06-04", "09:00"), + ("daily", "A", "morning", "2026-06-05", "09:00"), + ("daily", "A", "morning", "2026-06-06", "09:00"), + ("daily", "A", "morning", "2026-06-07", "09:00"), + ("daily", "A", "morning", "2026-06-15", "09:00"), + ("daily", "A", "morning", "2026-06-16", "09:00"), + ("daily", "A", "morning", "2026-06-17", "09:00"), + ("daily", "A", "morning", "2026-06-18", "09:00"), + ("daily", "A", "morning", "2026-06-19", "09:00"), + ("daily", "A", "morning", "2026-06-20", "09:00"), + ("daily", "A", "morning", "2026-06-21", "09:00"), + # daily C is present every day: all 28 days. + ("daily", "C", "morning", "2026-06-01", "09:00"), + ("daily", "C", "morning", "2026-06-02", "09:00"), + ("daily", "C", "morning", "2026-06-03", "09:00"), + ("daily", "C", "morning", "2026-06-04", "09:00"), + ("daily", "C", "morning", "2026-06-05", "09:00"), + ("daily", "C", "morning", "2026-06-06", "09:00"), + ("daily", "C", "morning", "2026-06-07", "09:00"), + ("daily", "C", "morning", "2026-06-08", "09:00"), + ("daily", "C", "morning", "2026-06-09", "09:00"), + ("daily", "C", "morning", "2026-06-10", "09:00"), + ("daily", "C", "morning", "2026-06-11", "09:00"), + ("daily", "C", "morning", "2026-06-12", "09:00"), + ("daily", "C", "morning", "2026-06-13", "09:00"), + ("daily", "C", "morning", "2026-06-14", "09:00"), + ("daily", "C", "morning", "2026-06-15", "09:00"), + ("daily", "C", "morning", "2026-06-16", "09:00"), + ("daily", "C", "morning", "2026-06-17", "09:00"), + ("daily", "C", "morning", "2026-06-18", "09:00"), + ("daily", "C", "morning", "2026-06-19", "09:00"), + ("daily", "C", "morning", "2026-06-20", "09:00"), + ("daily", "C", "morning", "2026-06-21", "09:00"), + ("daily", "C", "morning", "2026-06-22", "09:00"), + ("daily", "C", "morning", "2026-06-23", "09:00"), + ("daily", "C", "morning", "2026-06-24", "09:00"), + ("daily", "C", "morning", "2026-06-25", "09:00"), + ("daily", "C", "morning", "2026-06-26", "09:00"), + ("daily", "C", "morning", "2026-06-27", "09:00"), + ("daily", "C", "morning", "2026-06-28", "09:00"), + # weekly [B, C] Mondays, morning 07:00 + evening 19:00. + # B present weeks 1, 3 -> Mondays 06-08 and 06-22. + ("weekly", "B", "morning", "2026-06-08", "07:00"), + ("weekly", "B", "evening", "2026-06-08", "19:00"), + ("weekly", "B", "morning", "2026-06-22", "07:00"), + ("weekly", "B", "evening", "2026-06-22", "19:00"), + # C present every Monday: 06-01, 06-08, 06-15, 06-22. + ("weekly", "C", "morning", "2026-06-01", "07:00"), + ("weekly", "C", "evening", "2026-06-01", "19:00"), + ("weekly", "C", "morning", "2026-06-08", "07:00"), + ("weekly", "C", "evening", "2026-06-08", "19:00"), + ("weekly", "C", "morning", "2026-06-15", "07:00"), + ("weekly", "C", "evening", "2026-06-15", "19:00"), + ("weekly", "C", "morning", "2026-06-22", "07:00"), + ("weekly", "C", "evening", "2026-06-22", "19:00"), + # month_day [C] the 15th, morning 08:00. 2026-06-15 is the 15th. + ("month_day", "C", "morning", "2026-06-15", "08:00"), + # month_wd [C] 2nd Wednesday, morning 07:30. June 2026's 2nd + # Wednesday is 06-10 (the 1st is 06-03). + ("month_wd", "C", "morning", "2026-06-10", "07:30"), + # yearly [C] 1st of June, afternoon 12:00. Fires on 06-01. + ("yearly", "C", "afternoon", "2026-06-01", "12:00"), + # custom [A, C] Saturdays + Sundays, evening 20:00. + # A present weeks 0, 2 -> 06-06, 06-07, 06-20, 06-21. + ("custom", "A", "evening", "2026-06-06", "20:00"), + ("custom", "A", "evening", "2026-06-07", "20:00"), + ("custom", "A", "evening", "2026-06-20", "20:00"), + ("custom", "A", "evening", "2026-06-21", "20:00"), + # C present every weekend day. + ("custom", "C", "evening", "2026-06-06", "20:00"), + ("custom", "C", "evening", "2026-06-07", "20:00"), + ("custom", "C", "evening", "2026-06-13", "20:00"), + ("custom", "C", "evening", "2026-06-14", "20:00"), + ("custom", "C", "evening", "2026-06-20", "20:00"), + ("custom", "C", "evening", "2026-06-21", "20:00"), + ("custom", "C", "evening", "2026-06-27", "20:00"), + ("custom", "C", "evening", "2026-06-28", "20:00"), +] + +# The tuples that must disappear when Cleo (C) is overridden to ABSENT for +# 2026-06-04 .. 2026-06-06 (a mid-window stretch): her two daily chores and +# the Saturday-evening custom chore on 06-06. +_E2E_REMOVED = [ + ("daily", "C", "morning", "2026-06-04", "09:00"), + ("daily", "C", "morning", "2026-06-05", "09:00"), + ("daily", "C", "morning", "2026-06-06", "09:00"), + ("custom", "C", "evening", "2026-06-06", "20:00"), +] + + +def _resolve_e2e(raw, def_ids, child_ids) -> set: + """Substitute the oracle's definition/child slugs with runtime ids.""" + return { + (def_ids[slug], child_ids[child], window, due_date, due_time) + for slug, child, window, due_date, due_time in raw + } + + +async def _collect_keyed(database, child_ids, start, end) -> dict: + """Return {(definition_id, child_id, window, due_date): (id, due_time)}. + + The stable parts of an instance — its widened key, its physical id and + its due_time snapshot — with the refreshable ``generated_at`` stamp + deliberately excluded. Comparing two of these captures the real + re-run contract: no rows added/removed, ids stable (upsert refreshes + in place, never delete + reinsert), and due_time untouched. + """ + dao = QuestInstancesDao(database) + keyed = {} + for child_id in child_ids: + for record in await dao.list_by_date_range(child_id, start, end): + key = ( + record.definition_id, + record.child_id, + record.window, + record.due_date, + ) + keyed[key] = (record.id, record.due_time) + return keyed + + +async def _collect_records(database, child_ids, start, end) -> set: + """Return the full 7-field instance records over the children/range. + + Includes the ``generated_at`` stamp, so this is only used for children + that should NEVER be written (Ada/Bo during Cleo's override): their + records are truly byte-for-byte untouched, unlike the idempotency + re-run where the stamp legitimately refreshes. + """ + dao = QuestInstancesDao(database) + records = set() + for child_id in child_ids: + for record in await dao.list_by_date_range(child_id, start, end): + records.add( + ( + record.id, + record.definition_id, + record.child_id, + record.window, + record.due_date, + record.due_time, + record.generated_at, + ) + ) + return records + + +def test_materialize_end_to_end_six_rule_types(tmp_path, monkeypatch) -> None: + import custom_components.nestquest.materialize as materialize_module + + # Give each materialize run a DISTINCT batch stamp: production upserts + # rewrite generated_at on every re-run, so a shared pinned stamp would + # merely hide that rewrite. The idempotency assertion below therefore + # ignores generated_at and checks the stable columns only. + stamps = {"n": 0} + + def _distinct_stamp() -> str: + stamps["n"] += 1 + return f"2026-06-01T00:00:{stamps['n']:02d}+00:00" + + monkeypatch.setattr(materialize_module, "_now_stamp", _distinct_stamp) + + async def _body(database): + from custom_components.nestquest.dao_presence import ( + PresenceOverridesDao, + ) + from custom_components.nestquest.presence import PresenceSchedule + + children = ChildrenDao(database) + schedules = PresenceSchedulesDao(database) + overrides = PresenceOverridesDao(database) + + a = await children.create("Ada", NOW) # present weeks 0, 2 + b = await children.create("Bo", NOW) # present weeks 1, 3 + c = await children.create("Cleo", NOW) # no schedule: always present + + anchor = datetime.date.fromisoformat(_E2E_ANCHOR) + start_iso = anchor.isoformat() + end_iso = (anchor + datetime.timedelta(days=27)).isoformat() + + # Two-week custody with OPPOSITE weeks, encoded through the + # PresenceSchedule model on the fixed Monday anchor. + all_week = frozenset(range(7)) + a_schedule = PresenceSchedule(a.id, 2, anchor, {0: all_week, 1: frozenset()}) + b_schedule = PresenceSchedule(b.id, 2, anchor, {0: frozenset(), 1: all_week}) + await schedules.upsert_by_child(a.id, 2, start_iso, a_schedule.encode()) + await schedules.upsert_by_child(b.id, 2, start_iso, b_schedule.encode()) + + def_ids = {} + def_ids["daily"] = (await create_quest_definition( + database, + "Daily chore", + ScheduleRule(rule_type=RuleType.DAILY, start_date=start_iso), + [a.id, c.id], + [("morning", "09:00")], + )).definition.id + def_ids["weekly"] = (await create_quest_definition( + database, + "Weekly Monday chore", + ScheduleRule( + rule_type=RuleType.WEEKLY, + weekday_set={0}, + start_date=start_iso, + ), + [b.id, c.id], + [("morning", "07:00"), ("evening", "19:00")], + )).definition.id + def_ids["month_day"] = (await create_quest_definition( + database, + "Monthly 15th chore", + ScheduleRule( + rule_type=RuleType.MONTHLY_DAY, + day_of_month=15, + start_date=start_iso, + ), + [c.id], + [("morning", "08:00")], + )).definition.id + def_ids["month_wd"] = (await create_quest_definition( + database, + "Monthly 2nd-Wednesday chore", + ScheduleRule( + rule_type=RuleType.MONTHLY_WEEKDAY, + nth_weekday=2, + nth_weekday_weekday=2, + start_date=start_iso, + ), + [c.id], + [("morning", "07:30")], + )).definition.id + def_ids["yearly"] = (await create_quest_definition( + database, + "Yearly first-of-June chore", + ScheduleRule( + rule_type=RuleType.YEARLY, + month=anchor.month, + day_of_month=1, + start_date=start_iso, + ), + [c.id], + [("afternoon", "12:00")], + )).definition.id + def_ids["custom"] = (await create_quest_definition( + database, + "Weekend chore", + ScheduleRule( + rule_type=RuleType.CUSTOM_DAYS, + weekday_set={5, 6}, + start_date=start_iso, + ), + [a.id, c.id], + [("evening", "20:00")], + )).definition.id + + child_ids = {"A": a.id, "B": b.id, "C": c.id} + expected = _resolve_e2e(_E2E_EXPECTED, def_ids, child_ids) + removed = _resolve_e2e(_E2E_REMOVED, def_ids, child_ids) + all_children = (a.id, b.id, c.id) + + # First run: every (definition, child, window, date) tuple, exactly. + first = await materialize(database, start_iso, end_iso, today=anchor) + assert first == len(expected) + assert await _collect_instances( + database, all_children, start_iso, end_iso + ) == expected + + # Re-run contract: the upsert refreshes each existing row IN PLACE, + # so the widened key set, the physical ids and each due_time are all + # stable (only generated_at may refresh to the new run's stamp). + first_keyed = await _collect_keyed( + database, all_children, start_iso, end_iso + ) + second = await materialize(database, start_iso, end_iso, today=anchor) + assert second == len(expected) + assert await _collect_keyed( + database, all_children, start_iso, end_iso + ) == first_keyed + + # Capture Ada and Bo's complete records BEFORE the override, so we + # can prove they are untouched afterward. + a_before = await _collect_records(database, (a.id,), start_iso, end_iso) + b_before = await _collect_records(database, (b.id,), start_iso, end_iso) + + # Cleo is ABSENT 2026-06-04 .. 2026-06-06 (mid-window). + await overrides.create( + c.id, "2026-06-04", "2026-06-06", False, note="away" + ) + + # regenerate_for_child deletes + rebuilds CLEO ONLY (Ada and Bo are + # never touched). It rebuilds a 14-day rolling horizon though, so + # the child-scoped materialize below restores the remainder of the + # 28-day window without ever writing another child. + await regenerate_for_child(database, c.id, today=anchor) + await materialize( + database, start_iso, end_iso, child_ids=[c.id], today=anchor + ) + + # Ada and Bo are byte-for-byte untouched: id, snapshot columns and + # generated_at alike. + assert await _collect_records( + database, (a.id,), start_iso, end_iso + ) == a_before + assert await _collect_records( + database, (b.id,), start_iso, end_iso + ) == b_before + + # Exactly Cleo's override-covered tuples vanish; every other tuple + # survives. + remaining = await _collect_instances( + database, all_children, start_iso, end_iso + ) + assert remaining == expected - removed + assert remaining.isdisjoint(removed) + return first + + _with_db(tmp_path, "materialize-e2e.db")(_body) + + def test_regenerate_for_child_rejects_non_int_child_id(tmp_path) -> None: async def _body(database): for bad in (True, False, 1.0, "1"): From 3add2d2f484fda5707edfc964c639df50b90a886 Mon Sep 17 00:00:00 2001 From: Joshua Date: Thu, 17 Sep 2026 17:07:10 -0400 Subject: [PATCH 11/12] Thread configured horizon_days (Feature 07 integration fix) (#86) --- custom_components/nestquest/__init__.py | 68 +++++--- custom_components/nestquest/materialize.py | 50 +++++- .../nestquest/quest_definitions.py | 32 +++- tests/test_lifecycle.py | 125 ++++++++++++++ tests/test_materialize.py | 153 ++++++++++++++++++ tests/test_quest_definitions.py | 69 ++++++++ 6 files changed, 465 insertions(+), 32 deletions(-) diff --git a/custom_components/nestquest/__init__.py b/custom_components/nestquest/__init__.py index 6615679..8cc9b79 100644 --- a/custom_components/nestquest/__init__.py +++ b/custom_components/nestquest/__init__.py @@ -19,6 +19,7 @@ from homeassistant.helpers.event import async_track_time_change from .const import ( CONF_ADMIN_USER_IDS, CONF_DAY_ROLLOVER_TIME, + CONF_HORIZON_DAYS, DEFAULT_DAY_ROLLOVER_TIME, DEFAULT_HORIZON_DAYS, DOMAIN, @@ -273,10 +274,28 @@ def _day_rollover_hour_minute(options: dict[str, Any]) -> tuple[int, int]: return int(hour), int(minute) +def _configured_horizon_days(options: dict[str, Any]) -> int: + """Return the configured generation horizon, validated, else the default. + + The options flow already validates ``horizon_days`` as an integer >= 1, + but a stored ``entry.options`` dict could still carry a partial or + hand-edited value. Re-checking here — and falling back to + :data:`~.const.DEFAULT_HORIZON_DAYS` on a missing, non-int or sub-1 value + — keeps a malformed option from shrinking or exploding the materialized + horizon. + """ + value = options.get(CONF_HORIZON_DAYS, DEFAULT_HORIZON_DAYS) + if isinstance(value, bool) or not isinstance(value, int) or value < 1: + return DEFAULT_HORIZON_DAYS + return value + + async def _run_horizon_materialization( - hass: HomeAssistant, database: NestQuestDatabase + hass: HomeAssistant, + database: NestQuestDatabase, + horizon_days: int = DEFAULT_HORIZON_DAYS, ) -> None: - """Materialize the rolling horizon ``[today, today + DEFAULT_HORIZON_DAYS]``. + """Materialize the rolling horizon ``[today, today + horizon_days]``. "today" is computed in HA local time (``hass.config.time_zone``) rather than the system clock, so the horizon tracks the household's own day. @@ -285,13 +304,15 @@ async def _run_horizon_materialization( This is the ONE generation path: the startup backfill, the daily rollover listener and the ``regenerate`` service all funnel through it, so there is - no duplicated materialization logic. + no duplicated materialization logic. ``horizon_days`` sizes the window + and defaults to :data:`~.const.DEFAULT_HORIZON_DAYS`; callers thread the + entry's validated option value through. """ time_zone = ZoneInfo(hass.config.time_zone) today = datetime.datetime.now(time_zone).date() start_date = today.isoformat() end_date = ( - today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS) + today + datetime.timedelta(days=horizon_days) ).isoformat() await _materialize_run(database, start_date, end_date, today=today) @@ -304,31 +325,34 @@ def _register_day_rollover_listener( """Register the daily materialization listener and return its remover. Fires once per day at the configured local ``day_rollover_time`` and - materializes the rolling horizon ``[today, today + DEFAULT_HORIZON_DAYS]`` - through the shared :func:`_run_horizon_materialization` path. + materializes the rolling horizon ``[today, today + horizon_days]`` + through the shared :func:`_run_horizon_materialization` path, using the + entry's configured (validated) ``horizon_days``. """ hour, minute = _day_rollover_hour_minute(options) + horizon_days = _configured_horizon_days(options) async def _run_materialization(_now: datetime.datetime) -> None: - await _run_horizon_materialization(hass, database) + await _run_horizon_materialization(hass, database, horizon_days) return async_track_time_change( hass, _run_materialization, hour=hour, minute=minute ) -def _find_live_database(hass: HomeAssistant) -> NestQuestDatabase | None: - """Return a connected database from the active runtime data, or None. +def _find_live_runtime_data(hass: HomeAssistant) -> NestQuestRuntimeData | None: + """Return a live runtime record with a connected database, or None. The domain-global services (e.g. ``regenerate``) have no database of - their own; they resolve whichever config entry's database is currently - live at call time, so none of them can hold a stale handle to a closed - connection after another entry unloads. + their own; they resolve whichever config entry's runtime data is + currently live at call time, so none of them can hold a stale handle + to a closed connection after another entry unloads. The record also + carries that entry's options (e.g. the configured horizon). """ for runtime_data in hass.data.get(DOMAIN, {}).values(): database = getattr(runtime_data, "database", None) if database is not None and database.connected: - return database + return runtime_data return None @@ -349,14 +373,17 @@ def _register_regenerate_service(hass: HomeAssistant) -> None: return async def _regenerate(_call: Any) -> None: - database = _find_live_database(hass) - if database is None: + runtime_data = _find_live_runtime_data(hass) + if runtime_data is None: _LOGGER.warning( "NestQuest regenerate requested but no config entry has a " "live database; ignoring" ) return - await _run_horizon_materialization(hass, database) + horizon_days = _configured_horizon_days(runtime_data.options) + await _run_horizon_materialization( + hass, runtime_data.database, horizon_days + ) hass.services.async_register(DOMAIN, SERVICE_REGENERATE, _regenerate) @@ -456,9 +483,12 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: ) # Backfill any days the daily listener missed while the integration # was off (or freshly installed): run the one idempotent walk over - # [today, today + DEFAULT_HORIZON_DAYS] once at setup. This runs - # through the SAME path as the daily listener and regenerate service. - await _run_horizon_materialization(hass, database) + # [today, today + horizon_days] once at setup, using the entry's + # configured horizon. This runs through the SAME path as the daily + # listener and regenerate service. + await _run_horizon_materialization( + hass, database, _configured_horizon_days(dict(entry.options)) + ) _register_regenerate_service(hass) except BaseException: # A failure anywhere after the database is open must not leak the diff --git a/custom_components/nestquest/materialize.py b/custom_components/nestquest/materialize.py index e75c040..c32d3d9 100644 --- a/custom_components/nestquest/materialize.py +++ b/custom_components/nestquest/materialize.py @@ -44,7 +44,8 @@ definition edits / reassignment (Features 06/07); :func:`regenerate_for_child` covers a single child's presence change (Feature 09). Both delete the affected open instances at or after today — never a completed instance, never the past — and re-run the walk over the rolling horizon -``[today, today + DEFAULT_HORIZON_DAYS]``. +``[today, today + horizon_days]`` (``horizon_days`` defaults to +``DEFAULT_HORIZON_DAYS``). HOOK (Feature 09 presence services): the presence business layer that lands in Feature 09 MUST call :func:`regenerate_for_child` after EVERY @@ -83,6 +84,31 @@ def _now_stamp() -> str: ) +def _resolve_horizon_days(horizon_days: int | None) -> int: + """Validate a regeneration horizon and return the resolved day count. + + ``None`` resolves to :data:`~.const.DEFAULT_HORIZON_DAYS`. A bool + (SQLite would happily bind ``True``/``False`` onto an int), a non-int, + or a value below 1 raises ValueError naming the field — the validation + runs BEFORE any future-instance delete so a malformed horizon can never + delete rows it then fails to rebuild. A negative horizon would + otherwise delete first and only then be rejected by + :func:`materialize`'s inverted-range check; ``0``/``False`` would + silently shrink the window to a single day. + """ + if horizon_days is None: + return DEFAULT_HORIZON_DAYS + if ( + isinstance(horizon_days, bool) + or not isinstance(horizon_days, int) + or horizon_days < 1 + ): + raise ValueError( + f"horizon_days must be a positive integer, got {horizon_days!r}" + ) + return horizon_days + + def _decode_rule(snapshot) -> ScheduleRule: """Decode a snapshot's rule record back into a ScheduleRule. @@ -232,6 +258,7 @@ async def regenerate_for_definition( definition_id: int, *, today: datetime.date | None = None, + horizon_days: int | None = None, ) -> int: """Regenerate a definition's future instances after a config change. @@ -241,7 +268,7 @@ async def regenerate_for_definition( 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, + ``[today, today + 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 — @@ -251,14 +278,18 @@ async def regenerate_for_definition( range. ``today`` optionally pins a caller-resolved HA-local date (threaded into both the delete cutoff and the re-materialization) so a household time zone behind the host around midnight stays - consistent. Returns the number of instances the re-materialization + consistent. ``horizon_days`` sizes the re-materialization window; + when omitted it falls back to :data:`~.const.DEFAULT_HORIZON_DAYS`. + A bool, non-int, or sub-1 value raises ValueError BEFORE any delete. + Returns the number of instances the re-materialization upserted across the whole snapshot (idempotent — other active definitions' tuples refresh in place, never duplicate). """ + horizon = _resolve_horizon_days(horizon_days) today_date = _resolve_today(today) start_date = today_date.isoformat() end_date = ( - today_date + datetime.timedelta(days=DEFAULT_HORIZON_DAYS) + today_date + datetime.timedelta(days=horizon) ).isoformat() instances = QuestInstancesDao(database) @@ -286,6 +317,7 @@ async def regenerate_for_child( child_id: int, *, today: datetime.date | None = None, + horizon_days: int | None = None, ) -> int: """Regenerate a child's future instances after a presence change. @@ -299,7 +331,7 @@ async def regenerate_for_child( (never a completed instance, never the past) via :meth:`~.dao_instances.QuestInstancesDao.delete_future_uncompleted_for_child`, then the materialization walk re-runs over the rolling horizon - ``[today, today + DEFAULT_HORIZON_DAYS]`` scoped to that child only, + ``[today, today + horizon_days]`` scoped to that child only, so the child's now-absent/present dates re-materialize against today's presence WITHOUT touching any other child's rows. @@ -315,13 +347,17 @@ async def regenerate_for_child( execution-day anchor even when the call is queued across midnight. ``today`` optionally pins a caller-resolved HA-local date, threaded into the delete cutoff and the re-materialization for the same - host-vs-HA time-zone consistency. Returns the number of instances + host-vs-HA time-zone consistency. ``horizon_days`` sizes the + re-materialization window; when omitted it falls back to + :data:`~.const.DEFAULT_HORIZON_DAYS`. A bool, non-int, or sub-1 value + raises ValueError BEFORE any delete. Returns the number of instances the re-materialization upserted for the child across the whole snapshot. """ if isinstance(child_id, bool) or not isinstance(child_id, int): raise ValueError(f"child_id must be an integer, got {child_id!r}") + horizon = _resolve_horizon_days(horizon_days) today_date = _resolve_today(today) instances = QuestInstancesDao(database) @@ -335,7 +371,7 @@ async def regenerate_for_child( child_id, today_date.isoformat(), today=today ) start = datetime.date.fromisoformat(effective_cutoff) - end_date = (start + datetime.timedelta(days=DEFAULT_HORIZON_DAYS)).isoformat() + end_date = (start + datetime.timedelta(days=horizon)).isoformat() return await materialize( database, start.isoformat(), end_date, child_ids=[child_id], today=today ) diff --git a/custom_components/nestquest/quest_definitions.py b/custom_components/nestquest/quest_definitions.py index 86ada89..d301aa4 100644 --- a/custom_components/nestquest/quest_definitions.py +++ b/custom_components/nestquest/quest_definitions.py @@ -272,6 +272,7 @@ async def edit_quest_definition( rule: ScheduleRule | object = _UNSET, windows: list[str | tuple[str, str | None]] | object = _UNSET, today: datetime.date | None = None, + horizon_days: int | None = None, ) -> CreatedQuestDefinition: """Edit a quest definition's metadata, rule and windows atomically. @@ -299,7 +300,10 @@ async def edit_quest_definition( into :func:`~.materialize.regenerate_for_definition`); it is a plain ``datetime.date`` with no HA import here — the Feature 09 service supplies the hass-derived value. When omitted, regeneration falls - back to the host clock, same as before. + back to the host clock, same as before. ``horizon_days`` sizes the + regeneration's re-materialization window the same way (threaded into + :func:`~.materialize.regenerate_for_definition`); it defaults to + :data:`~.const.DEFAULT_HORIZON_DAYS` when omitted. """ _validate_definition_id(definition_id) @@ -350,7 +354,9 @@ async def edit_quest_definition( schedule_rule_storage_from_record(snapshot.rule) ) - await regenerate_for_definition(database, definition_id, today=today) + await regenerate_for_definition( + database, definition_id, today=today, horizon_days=horizon_days + ) return CreatedQuestDefinition( definition=snapshot.definition, @@ -366,6 +372,7 @@ async def assign_child( child_id: int, *, today: datetime.date | None = None, + horizon_days: int | None = None, ) -> list[ChildRecord]: """Assign ``child_id`` to the definition (idempotent). @@ -382,6 +389,8 @@ async def assign_child( ``today`` optionally pins the caller-resolved HA-local date threaded into the assignment's regeneration (see :func:`edit_quest_definition`). + ``horizon_days`` optionally sizes the regeneration's re-materialization + window the same way. """ _validate_definition_id(definition_id) _validate_child_id(child_id) @@ -397,7 +406,9 @@ 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, today=today) + await regenerate_for_definition( + database, definition_id, today=today, horizon_days=horizon_days + ) return assignees @@ -407,6 +418,7 @@ async def unassign_child( child_id: int, *, today: datetime.date | None = None, + horizon_days: int | None = None, ) -> bool: """Unassign ``child_id`` from the definition; True when removed. @@ -419,6 +431,8 @@ async def unassign_child( ``today`` optionally pins the caller-resolved HA-local date threaded into the unassignment's regeneration (see :func:`edit_quest_definition`). + ``horizon_days`` optionally sizes the regeneration's re-materialization + window the same way. """ _validate_definition_id(definition_id) _validate_child_id(child_id) @@ -429,7 +443,9 @@ async def unassign_child( f"definition_id: quest definition {definition_id} does not exist" ) removed = await dao.remove_assignee(definition_id, child_id) - await regenerate_for_definition(database, definition_id, today=today) + await regenerate_for_definition( + database, definition_id, today=today, horizon_days=horizon_days + ) return removed @@ -439,6 +455,7 @@ async def set_quest_definition_active( is_active: bool, *, today: datetime.date | None = None, + horizon_days: int | None = None, ) -> CreatedQuestDefinition: """Deactivate or reactivate a definition; returns the updated view. @@ -464,7 +481,8 @@ async def set_quest_definition_active( ``today`` optionally pins the caller-resolved HA-local date threaded into the activation change's regeneration (see - :func:`edit_quest_definition`). + :func:`edit_quest_definition`). ``horizon_days`` optionally sizes the + regeneration's re-materialization window the same way. """ _validate_definition_id(definition_id) if not isinstance(is_active, bool): @@ -488,7 +506,9 @@ 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, today=today) + await regenerate_for_definition( + database, definition_id, today=today, horizon_days=horizon_days + ) return CreatedQuestDefinition( definition=updated, rule=rule, diff --git a/tests/test_lifecycle.py b/tests/test_lifecycle.py index 68be6ce..b81f3f9 100644 --- a/tests/test_lifecycle.py +++ b/tests/test_lifecycle.py @@ -10,6 +10,7 @@ from conftest import make_config_entry, make_hass, wire_entry_to_registry from custom_components.nestquest import DOMAIN, async_setup_entry, async_unload_entry from custom_components.nestquest.const import ( CONF_DAY_ROLLOVER_TIME, + CONF_HORIZON_DAYS, DEFAULT_HORIZON_DAYS, SERVICE_REGENERATE, DOMAIN as DOMAIN_CONST, @@ -351,6 +352,39 @@ async def test_day_rollover_listener_materializes_ha_local_horizon( ) +async def test_day_rollover_uses_configured_horizon_days(hass, make_entry) -> None: + """The daily run materializes only the configured horizon window.""" + import datetime + + from custom_components.nestquest.dao_instances import QuestInstancesDao + + entry = _wire(make_entry(options={CONF_HORIZON_DAYS: 5}), hass.registry) + assert await async_setup_entry(hass, entry) is True + database = entry.runtime_data.database + + # A definition created AFTER setup has no instances yet (create does not + # materialize); firing the daily listener must generate only the horizon. + child_id = await _seed_daily_child_and_definition( + database, _local_today(hass) + ) + await hass.time_change.fire() + + today = _local_today(hass) + end = (today + datetime.timedelta(days=5)).isoformat() + records = await QuestInstancesDao(database).list_by_date_range( + child_id, today.isoformat(), end + ) + assert len(records) == 6 + + beyond = (today + datetime.timedelta(days=6)).isoformat() + beyond_end = ( + today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS) + ).isoformat() + assert await QuestInstancesDao(database).list_by_date_range( + child_id, beyond, beyond_end + ) == [] + + def test_day_rollover_time_change_tracker_rejects_local_kwarg() -> None: """The fake async_track_time_change mirrors HA 2024.6 (no ``local`` kwarg). @@ -447,6 +481,63 @@ async def test_setup_backfills_instances_over_horizon(hass, make_entry) -> None: assert len(records) == DEFAULT_HORIZON_DAYS + 1 +async def test_setup_backfills_configured_horizon_days(hass, make_entry) -> None: + """Setup backfills only the configured (non-default) horizon window.""" + import datetime + + from custom_components.nestquest.dao_instances import QuestInstancesDao + from custom_components.nestquest.db import NestQuestDatabase + from custom_components.nestquest.migrations import apply_migrations + from custom_components.nestquest.store import async_get_db_path + + db_path = await async_get_db_path(hass) + pre = NestQuestDatabase(hass) + await pre.open(db_path) + await apply_migrations(pre) + child_id = await _seed_daily_child_and_definition(pre, _local_today(hass)) + await pre.close() + + entry = _wire( + make_entry(options={CONF_HORIZON_DAYS: 5}), hass.registry + ) + assert await async_setup_entry(hass, entry) is True + + today = _local_today(hass) + end = (today + datetime.timedelta(days=5)).isoformat() + records = await QuestInstancesDao( + entry.runtime_data.database + ).list_by_date_range(child_id, today.isoformat(), end) + assert len(records) == 6 + + beyond = (today + datetime.timedelta(days=6)).isoformat() + beyond_end = ( + today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS) + ).isoformat() + assert await QuestInstancesDao( + entry.runtime_data.database + ).list_by_date_range(child_id, beyond, beyond_end) == [] + + +def test_configured_horizon_days_validates_and_falls_back() -> None: + """The configured horizon is validated; a malformed value uses the default.""" + import custom_components.nestquest as nestquest + + assert nestquest._configured_horizon_days({CONF_HORIZON_DAYS: 5}) == 5 + assert nestquest._configured_horizon_days({}) == DEFAULT_HORIZON_DAYS + assert ( + nestquest._configured_horizon_days({CONF_HORIZON_DAYS: True}) + == DEFAULT_HORIZON_DAYS + ) + assert ( + nestquest._configured_horizon_days({CONF_HORIZON_DAYS: "5"}) + == DEFAULT_HORIZON_DAYS + ) + assert ( + nestquest._configured_horizon_days({CONF_HORIZON_DAYS: 0}) + == DEFAULT_HORIZON_DAYS + ) + + async def test_regenerate_service_registered_and_materializes( hass, make_entry ) -> None: @@ -474,6 +565,40 @@ async def test_regenerate_service_registered_and_materializes( assert len(records) == DEFAULT_HORIZON_DAYS + 1 +async def test_regenerate_service_uses_configured_horizon_days( + hass, make_entry +) -> None: + """``nestquest.regenerate`` regenerates only the configured horizon.""" + import datetime + + from custom_components.nestquest.dao_instances import QuestInstancesDao + + entry = _wire( + make_entry(options={CONF_HORIZON_DAYS: 5}), hass.registry + ) + assert await async_setup_entry(hass, entry) is True + + database = entry.runtime_data.database + child_id = await _seed_daily_child_and_definition(database, _local_today(hass)) + + await hass.services.call(DOMAIN, SERVICE_REGENERATE) + + today = _local_today(hass) + end = (today + datetime.timedelta(days=5)).isoformat() + records = await QuestInstancesDao(database).list_by_date_range( + child_id, today.isoformat(), end + ) + assert len(records) == 6 + + beyond = (today + datetime.timedelta(days=6)).isoformat() + beyond_end = ( + today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS) + ).isoformat() + assert await QuestInstancesDao(database).list_by_date_range( + child_id, beyond, beyond_end + ) == [] + + async def test_unload_deregisters_regenerate_service(hass, make_entry) -> None: """Unload removes the ``regenerate`` service so no stale entry leaks.""" entry = _wire(make_entry(), hass.registry) diff --git a/tests/test_materialize.py b/tests/test_materialize.py index 390b3ea..2135559 100644 --- a/tests/test_materialize.py +++ b/tests/test_materialize.py @@ -7,6 +7,7 @@ import datetime import pytest from custom_components.nestquest.dao_children import ChildrenDao +from custom_components.nestquest.const import DEFAULT_HORIZON_DAYS from custom_components.nestquest.dao_instances import ( CompletionEventsDao, QuestInstancesDao, @@ -1113,6 +1114,158 @@ def test_regenerate_for_child_is_child_scoped(tmp_path) -> None: _with_db(tmp_path, "regenerate-child-scoped.db")(_body) +def test_regenerate_for_definition_respects_horizon_days(tmp_path) -> None: + """A non-default horizon_days sizes the definition's rebuild window.""" + async def _body(database): + children = ChildrenDao(database) + child = await children.create("Ada", NOW) + + today = datetime.date.today() + 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")], + ) + + # ``create`` does not materialize; regeneration over a 5-day horizon + # must build exactly today..today+5 (six daily rows). + await regenerate_for_definition( + database, created.definition.id, horizon_days=5 + ) + + dao = QuestInstancesDao(database) + horizon_end = today + datetime.timedelta(days=5) + records = await dao.list_by_date_range( + child.id, start_iso, horizon_end.isoformat() + ) + assert len(records) == 6 + + # The window is BOUNDED: nothing must exist at day 6 or later (a + # hard-coded 14-day default would still emit rows there). + beyond = (today + datetime.timedelta(days=6)).isoformat() + beyond_end = ( + today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS) + ).isoformat() + assert await dao.list_by_date_range(child.id, beyond, beyond_end) == [] + return None + + _with_db(tmp_path, "regenerate-def-horizon.db")(_body) + + +def test_regenerate_for_child_respects_horizon_days(tmp_path) -> None: + """A non-default horizon_days sizes the child's rebuild window.""" + async def _body(database): + children = ChildrenDao(database) + child = await children.create("Ada", NOW) + + today = datetime.date.today() + start_iso = today.isoformat() + + await create_quest_definition( + database, + "Daily chore", + ScheduleRule(rule_type=RuleType.DAILY, start_date=start_iso), + [child.id], + [("morning", "09:00")], + ) + + await regenerate_for_child(database, child.id, horizon_days=5) + + dao = QuestInstancesDao(database) + horizon_end = today + datetime.timedelta(days=5) + records = await dao.list_by_date_range( + child.id, start_iso, horizon_end.isoformat() + ) + assert len(records) == 6 + + # Bounded window: nothing must exist at day 6 or later. + beyond = (today + datetime.timedelta(days=6)).isoformat() + beyond_end = ( + today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS) + ).isoformat() + assert await dao.list_by_date_range(child.id, beyond, beyond_end) == [] + return None + + _with_db(tmp_path, "regenerate-child-horizon.db")(_body) + + +@pytest.mark.parametrize("bad", [-1, 0, True, False, "5", 1.5]) +def test_regenerate_for_definition_rejects_invalid_horizon_before_delete( + tmp_path, bad +) -> None: + """A malformed horizon fails BEFORE any future-instance delete.""" + async def _body(database): + children = ChildrenDao(database) + child = await children.create("Ada", NOW) + + today = datetime.date.today() + start_iso = today.isoformat() + horizon_end = today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS) + + created = await create_quest_definition( + database, + "Daily chore", + ScheduleRule(rule_type=RuleType.DAILY, start_date=start_iso), + [child.id], + [("morning", "09:00")], + ) + dao = QuestInstancesDao(database) + await materialize(database, start_iso, horizon_end.isoformat()) + + with pytest.raises(ValueError, match="horizon_days"): + await regenerate_for_definition( + database, created.definition.id, horizon_days=bad + ) + + # The refused call deleted nothing: the seeded horizon survives. + records = await dao.list_by_date_range( + child.id, start_iso, horizon_end.isoformat() + ) + assert len(records) == DEFAULT_HORIZON_DAYS + 1 + return None + + _with_db(tmp_path, "regenerate-def-horizon-invalid.db")(_body) + + +@pytest.mark.parametrize("bad", [-1, 0, True, False, "5", 1.5]) +def test_regenerate_for_child_rejects_invalid_horizon_before_delete( + tmp_path, bad +) -> None: + """A malformed child horizon fails BEFORE any future-instance delete.""" + async def _body(database): + children = ChildrenDao(database) + child = await children.create("Ada", NOW) + + today = datetime.date.today() + start_iso = today.isoformat() + horizon_end = today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS) + + await create_quest_definition( + database, + "Daily chore", + ScheduleRule(rule_type=RuleType.DAILY, start_date=start_iso), + [child.id], + [("morning", "09:00")], + ) + dao = QuestInstancesDao(database) + await materialize(database, start_iso, horizon_end.isoformat()) + + with pytest.raises(ValueError, match="horizon_days"): + await regenerate_for_child(database, child.id, horizon_days=bad) + + records = await dao.list_by_date_range( + child.id, start_iso, horizon_end.isoformat() + ) + assert len(records) == DEFAULT_HORIZON_DAYS + 1 + return None + + _with_db(tmp_path, "regenerate-child-horizon-invalid.db")(_body) + + #: Fixed end-to-end anchor: 2026-06-01 is a Monday AND the 1st of its month, #: so the two-week custody cycle is week-aligned and the yearly rule's #: "1st of the anchor month" is GUARANTEED to fire inside the 28-day window diff --git a/tests/test_quest_definitions.py b/tests/test_quest_definitions.py index 2770113..a755670 100644 --- a/tests/test_quest_definitions.py +++ b/tests/test_quest_definitions.py @@ -8,6 +8,7 @@ import re import pytest from custom_components.nestquest.dao_children import ChildrenDao +from custom_components.nestquest.const import DEFAULT_HORIZON_DAYS from custom_components.nestquest.dao_instances import ( CompletionEventsDao, QuestInstancesDao, @@ -1938,3 +1939,71 @@ def test_full_lifecycle_preserves_completed_and_history(tmp_path) -> None: return reactivated _with_db(tmp_path, "full-lifecycle.db")(_body) + + +@pytest.mark.parametrize( + "mode", + ["edit", "assign", "unassign", "set_active"], +) +def test_wrappers_forward_horizon_days(tmp_path, mode) -> None: + """The four change wrappers forward a non-default horizon_days. + + Each wrapper threads ``horizon_days`` into the regeneration it triggers; + passing 5 here must bound the regenerated window to today..today+5 — + proving the forwarding argument is actually honored, not dropped. + """ + async def _body(database, children, rules, definitions, child): + created = await create_quest_definition( + database, + "Chore", + ScheduleRule(rule_type=RuleType.DAILY, start_date=_today_iso()), + [child.id], + ["morning"], + ) + definition_id = created.definition.id + instances = QuestInstancesDao(database) + + if mode == "edit": + await edit_quest_definition( + database, + definition_id, + windows=[("morning", "10:30")], + horizon_days=5, + ) + tracked = child + elif mode == "assign": + bo = await children.create("Bo", NOW) + await assign_child(database, definition_id, bo.id, horizon_days=5) + tracked = bo + elif mode == "unassign": + # The remaining assignee (``child``) must be regenerated over + # the 5-day window after ``bo`` is removed. + bo = await children.create("Bo", NOW) + await assign_child(database, definition_id, bo.id) + await unassign_child( + database, definition_id, bo.id, horizon_days=5 + ) + tracked = child + else: # set_active + await set_quest_definition_active( + database, definition_id, True, horizon_days=5 + ) + tracked = child + + today = datetime.date.today() + end = (today + datetime.timedelta(days=5)).isoformat() + records = await instances.list_by_date_range( + tracked.id, today.isoformat(), end + ) + assert len(records) == 6 + + beyond = (today + datetime.timedelta(days=6)).isoformat() + beyond_end = ( + today + datetime.timedelta(days=DEFAULT_HORIZON_DAYS) + ).isoformat() + assert await instances.list_by_date_range( + tracked.id, beyond, beyond_end + ) == [] + return None + + _with_db(tmp_path, f"wrapper-horizon-{mode}.db")(_body) From 6e32761b0c6504e0dd2fc4dadefb4d1ae1a852fd Mon Sep 17 00:00:00 2001 From: Joshua Date: Thu, 17 Sep 2026 17:17:57 -0400 Subject: [PATCH 12/12] Guard against stale due_time overwrite (Feature 07 fix) (#87) --- custom_components/nestquest/dao_instances.py | 25 +++++---- tests/test_materialize.py | 57 +++++++++++++++++--- 2 files changed, 66 insertions(+), 16 deletions(-) diff --git a/custom_components/nestquest/dao_instances.py b/custom_components/nestquest/dao_instances.py index 94f05b9..e483d6c 100644 --- a/custom_components/nestquest/dao_instances.py +++ b/custom_components/nestquest/dao_instances.py @@ -344,14 +344,20 @@ 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). + is skipped); + - the window's live ``due_time`` still EQUALS the caller-supplied + ``due_time`` (NULL-safe: NULL == NULL counts as equal; a window + with no due time matches a snapshot with no due time). A + window-time edit landed after the input snapshot makes the + caller's ``due_time`` stale, and writing it would overwrite the + newer due_time a concurrent regeneration already upserted — so + the tuple is skipped instead of written. - 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. + The due_time check closes the one check-then-insert gap this + method previously left open: it re-validated every OTHER + precondition but then wrote the snapshot's ``due_time`` + unconditionally, so a stale materialization could clobber a + newer, concurrently-regenerated due_time. Returns the stored instance, or ``None`` when a precondition failed — the tuple no longer materializable, which the walk SKIPS @@ -384,8 +390,9 @@ class QuestInstancesDao: "JOIN quest_definition_windows w " "ON w.definition_id = d.id " "WHERE d.id = ? AND a.child_id = ? AND w.window = ? " - "AND d.is_active = 1 AND c.is_active = 1 LIMIT 1", - (definition_id, child_id, window), + "AND d.is_active = 1 AND c.is_active = 1 " + "AND w.due_time IS ? LIMIT 1", + (definition_id, child_id, window, due_time), ) if still_valid is None: # A config change invalidated the tuple since the diff --git a/tests/test_materialize.py b/tests/test_materialize.py index 2135559..3b1ea2f 100644 --- a/tests/test_materialize.py +++ b/tests/test_materialize.py @@ -545,7 +545,7 @@ 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: +def test_materialize_skips_stale_due_time_mid_walk(tmp_path) -> None: async def _body(database): children = ChildrenDao(database) child = await children.create("Ada", NOW) @@ -566,21 +566,64 @@ def test_materialize_stores_snapshot_due_time_not_live(tmp_path) -> None: ) # 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. + # snapshot and the insert. The walk's snapshotted 09:00 no + # longer matches the live 10:15, so the tuple is SKIPPED rather + # than writing a due_time that has already been superseded. count = await _materialize_with_change( database, start_iso, end_iso, _edit_due_time ) - assert count == 3 + assert count == 0 + assert await QuestInstancesDao(database).list_by_date_range( + child.id, start_iso, end_iso + ) == [] + return count + + _with_db(tmp_path, "materialize-stale-due-time.db")(_body) + + +def test_materialize_does_not_overwrite_regenerated_due_time(tmp_path) -> None: + async def _body(database): + children = ChildrenDao(database) + child = await children.create("Ada", NOW) + + today = datetime.date.today() + start_iso = today.isoformat() + end_iso = (today + 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_and_regenerate(): + # A window-time edit + regeneration land after the stale walk + # read its input snapshot: the window moves to 10:15 and the + # regeneration upserts the newer due_time across the horizon. + await QuestDefinitionsDao(database).upsert_window( + created.definition.id, "morning", due_time="10:15" + ) + await regenerate_for_definition(database, created.definition.id) + + # The stale walk (snapshot due_time 09:00) then resumes. It must + # SKIP these tuples — its snapshotted 09:00 no longer matches the + # live 10:15 — so the regeneration's newer due_time survives and + # is NOT overwritten by the stale value. + count = await _materialize_with_change( + database, start_iso, end_iso, _edit_and_regenerate + ) + assert count == 0 + 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"} + assert {r.due_time for r in records} == {"10:15"} return records - _with_db(tmp_path, "materialize-snapshot-due-time.db")(_body) + _with_db(tmp_path, "materialize-regenerated-due-time.db")(_body) def test_materialize_honours_overrides(tmp_path) -> None: