test: race coverage for list_definitions_for_child and list_active_definitions (F74-002)

Fix quest-definition query snapshot coherence and edit error field naming
Read definitions, rules, assignees and windows in one locked transaction via DAO snapshot-list methods so concurrent edits cannot interleave; re-raise edit's missing-definition error with a definition_id field prefix.
2026-09-17 12:04:50 -04:00 · 2026-09-17 11:55:35 -04:00
10 changed files with 51 additions and 1949 deletions
@@ -2,30 +2,19 @@
 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 (
+from .const import CONF_ADMIN_USER_IDS, DOMAIN, LOGGER
    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
@@ -235,7 +224,6 @@ 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]:
@@ -260,48 +248,6 @@ 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
@@ -384,9 +330,6 @@ 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
@@ -395,7 +338,6 @@ 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
@@ -412,29 +354,14 @@ 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:
-            for remove_listener in (
+            if remove_update_listener is not None:
-                remove_update_listener,
+                result = remove_update_listener()
-                remove_time_change_listener,
+                if inspect.isawaitable(result):
-            ):
+                    await result
-                if remove_listener is None:
+        except BaseException as err:
-                    continue
+            unload_error = err
                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:
@@ -47,18 +47,6 @@ 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.
@@ -215,7 +203,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 = _today().isoformat()
+        today = datetime.date.today().isoformat()
        if due_date < today:
            raise ValueError(
                f"instances are never generated in the past: due_date "
@@ -226,7 +214,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 = _today().isoformat()
+                today = datetime.date.today().isoformat()
                if due_date < today:
                    raise ValueError(
                        f"instances are never generated in the past: "
@@ -289,121 +277,6 @@ 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 (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)
        # 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}"
            )
        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}"
                    )
                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:
                    # 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) "
                    "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,
@@ -487,7 +360,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 = _today().isoformat()
+                today = datetime.date.today().isoformat()
                effective_cutoff = max(cutoff_date, today)
                result = await self._database.execute(
                    "DELETE FROM quest_instances WHERE definition_id = ? "
@@ -499,49 +372,6 @@ 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.
@@ -276,59 +276,4 @@ class PresenceOverridesDao:
            "DELETE FROM presence_overrides WHERE id = ?",
            (override_id,),
        )
-        return result.rowcount > 0
+        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
@@ -951,21 +951,6 @@ 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]:
@@ -1204,47 +1189,4 @@ class QuestDefinitionsDao:
        if not child[0]:
            raise ValueError(
                f"child {child_id} is inactive and cannot be assigned"
-            )
+            )
 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
@@ -1,302 +0,0 @@
 """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) is a separate later
 task and lives 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; 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, _today
 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,
    *,
    child_ids: list[int] | None = None,
 ) -> 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).
    ``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
    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)
    child_filter = None if child_ids is None else set(child_ids)
    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 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:
                    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
 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 —
    :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 = _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]
    )
@@ -58,7 +58,6 @@ from .dao_rules import (
    schedule_rule_to_storage,
 )
 from .db import NestQuestDatabase
 from .materialize import regenerate_for_definition
 from .recurrence import ScheduleRule, occurs_on
@@ -342,8 +341,6 @@ 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,
@@ -374,7 +371,7 @@ async def assign_child(
    _validate_child_id(child_id)
    dao = QuestDefinitionsDao(database)
    try:
-        assignees = await dao.add_assignee(definition_id, child_id)
+        return 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.
@@ -384,8 +381,6 @@ 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(
@@ -410,9 +405,7 @@ async def unassign_child(
        raise ValueError(
            f"definition_id: quest definition {definition_id} does not exist"
        )
-    removed = await dao.remove_assignee(definition_id, child_id)
+    return await dao.remove_assignee(definition_id, child_id)
    await regenerate_for_definition(database, definition_id)
    return removed
 async def set_quest_definition_active(
@@ -464,7 +457,6 @@ 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,
@@ -44,7 +44,6 @@ _HA_MODULES = (
    "homeassistant.exceptions",
    "homeassistant.helpers",
    "homeassistant.helpers.config_validation",
    "homeassistant.helpers.event",
 )
 def _find_spec(name: str):
@@ -84,7 +83,6 @@ 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}"])
@@ -244,69 +242,6 @@ 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,
@@ -368,10 +303,6 @@ 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)
@@ -8,11 +8,7 @@ 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 (
+from custom_components.nestquest.const import DOMAIN as DOMAIN_CONST
    CONF_DAY_ROLLOVER_TIME,
    DEFAULT_HORIZON_DAYS,
    DOMAIN as DOMAIN_CONST,
 )
 def _wire(entry, registry):
@@ -241,128 +237,6 @@ 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:
@@ -1279,19 +1279,11 @@ 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_preserves_completed_and_history(
+def test_set_quest_definition_active_never_deletes_or_touches_instances(
    tmp_path,
 ) -> None:
-    """Deactivation stops future generation; reactivation resumes it.
+    """Deactivation hides the definition but deletes nothing, and
-
+    pre-existing instances and completion history survive unchanged."""
    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"]
@@ -1318,6 +1310,9 @@ def test_set_quest_definition_active_preserves_completed_and_history(
            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"
        )
@@ -1337,25 +1332,29 @@ def test_set_quest_definition_active_preserves_completed_and_history(
            definition_id
        )] == ["morning"]
        # The pre-existing instance and completion event survive with
-        # unchanged ids and contents, and no new event rows appeared.
+        # unchanged ids and contents, and no new 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-preserves-completed.db")(_body)
+    _with_db(tmp_path, "set-active-never-deletes.db")(_body)
 def _gated_hass(
@@ -1842,15 +1841,16 @@ def test_list_active_definitions_snapshot_coherent_under_racing_edit(
 # ---------------------------------------------------------------------------
-def test_full_lifecycle_preserves_completed_and_history(tmp_path) -> None:
+def test_full_lifecycle_never_touches_instances_or_history(tmp_path) -> None:
-    """End-to-end lifecycle preserves completed rows and append-only history.
+    """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.
-    Each stage (create -> edit rule/windows -> assign a second child ->
+    A real instance and completion event are seeded before the lifecycle
-    unassign the original child -> deactivate -> reactivate) now
+    begins, so each stage is checked against pre-existing data being
-    regenerates the definition's future OPEN instances (Feature 07
+    mutated (not an empty table staying empty): create -> edit
-    wiring), but a pre-existing COMPLETED instance and its completion
+    rule/windows -> assign a second child -> unassign the original child
-    events survive every stage unchanged — the completed row keeps its
+    -> deactivate -> reactivate.
    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,18 +1876,24 @@ def test_full_lifecycle_preserves_completed_and_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_preserved():
+        async def _assert_history_untouched():
            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_preserved()
+        await _assert_history_untouched()
        # Edit rule and windows together; the definition's rule row is
        # rewritten in place and its window set replaced.
@@ -1908,32 +1914,32 @@ def test_full_lifecycle_preserves_completed_and_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_preserved()
+        await _assert_history_untouched()
        # 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_preserved()
+        await _assert_history_untouched()
        # 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_preserved()
+        await _assert_history_untouched()
        # 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_preserved()
+        await _assert_history_untouched()
        reactivated = await set_quest_definition_active(
            database, definition_id, True
        )
        assert reactivated.definition.is_active is True
-        await _assert_history_preserved()
+        await _assert_history_untouched()
        return reactivated
Author	SHA1	Message	Date
overseer	d09ac91ade	test: race coverage for list_definitions_for_child and list_active_definitions (F74-002)	2026-09-17 12:04:50 -04:00
overseer	e7daecf970	Fix quest-definition query snapshot coherence and edit error field naming Read definitions, rules, assignees and windows in one locked transaction via DAO snapshot-list methods so concurrent edits cannot interleave; re-raise edit's missing-definition error with a definition_id field prefix.	2026-09-17 11:55:35 -04:00