fix: return assignee roster within the lock (NQ-69-01)

custom_components/nestquest/dao_rules.py

@@ -907,66 +907,6 @@ class QuestDefinitionsDao:
         )
         return [_definition_from_row(row) for row in rows]
 
-    async def _snapshots_for(
-        self, definitions: list[QuestDefinitionRecord]
-    ) -> list[QuestDefinitionSnapshot]:
-        """Read each definition's rule, assignees and windows.
-
-        MUST be called inside the connection lock (and ideally a
-        transaction) so the definitions handed in and the rows this
-        reads back cannot be raced by a concurrent edit between list
-        and bundle.  Each rule is read through
-        :class:`ScheduleRulesDao` on the same connection, so the
-        snapshot's rule record is the one the definition references AT
-        the moment of the locked read.
-        """
-        rules_dao = ScheduleRulesDao(self._database)
-        snapshots: list[QuestDefinitionSnapshot] = []
-        for definition in definitions:
-            rule_record = await rules_dao.get(definition.schedule_rule_id)
-            assert rule_record is not None
-            snapshots.append(
-                QuestDefinitionSnapshot(
-                    definition=definition,
-                    rule=rule_record,
-                    assignees=await self.list_assignees(definition.id),
-                    windows=await self.list_windows(definition.id),
-                )
-            )
-        return snapshots
-
-    async def list_snapshots_active(self) -> list[QuestDefinitionSnapshot]:
-        """Return every active definition with its rule, assignees and
-        windows, all read inside one locked transaction.
-
-        The definitive list and each definition's linked rows are read
-        in a single BEGIN..COMMIT span under the connection lock, so a
-        concurrent atomic edit cannot interleave between the definition
-        list and the bundle read: the caller receives a coherent
-        snapshot (each definition's rule, assignees and windows as they
-        were at one instant), never a mixed old-rule/new-windows state.
-        """
-        async with _connection_lock(self._database):
-            async with self._database.transaction():
-                definitions = await self.list_active()
-                return await self._snapshots_for(definitions)
-
-    async def list_snapshots_by_child(
-        self, child_id: int
-    ) -> list[QuestDefinitionSnapshot]:
-        """Return each definition assigned to ``child_id`` with its rule,
-        assignees and windows, all read inside one locked transaction.
-
-        Mirrors :meth:`list_snapshots_active` but selects by assignment
-        (``list_by_child`` order: newest first).  Locks read-and-bundle
-        into one transaction so the bundle never mixes one edit's
-        definition with another's windows.
-        """
-        async with _connection_lock(self._database):
-            async with self._database.transaction():
-                definitions = await self.list_by_child(child_id)
-                return await self._snapshots_for(definitions)
-
     async def update(
         self,
         definition_id: int,

custom_components/nestquest/quest_definitions.py

@@ -3,11 +3,10 @@
 Sits between the Feature 09 service gate and the typed
 :class:`~.dao_rules.QuestDefinitionsDao`: callers get validation and the
 all-or-nothing create/edit here, and never touch the DAO SQL directly.
-:func:`create_quest_definition` is the create path,
+:func:`create_quest_definition` is the create path and
-:func:`edit_quest_definition` the edit path, :func:`assign_child` and
+:func:`edit_quest_definition` the edit path; :func:`assign_child` and
-:func:`unassign_child` the assignment paths, and
+:func:`unassign_child` are the assignment paths.  There is no permission
-:func:`set_quest_definition_active` the deactivate/reactivate path.
+check here (that is Feature 09).
-There is no permission check here (that is Feature 09).
 
 Validation policy mirrors :mod:`.children`:
 
@@ -41,16 +40,13 @@ from __future__ import annotations
 import datetime
 from dataclasses import dataclass
 
-from .dao_children import ChildRecord, _connection_lock
+from .dao_children import ChildRecord
 from .dao_rules import (
     QuestDefinitionRecord,
-    QuestDefinitionSnapshot,
     QuestDefinitionWindowRecord,
     QuestDefinitionsDao,
     ScheduleRuleStorage,
-    ScheduleRulesDao,
     _UNSET,
-    _validate_date,
     _validate_time,
     _validate_window,
     schedule_rule_from_storage,
@@ -58,12 +54,12 @@ from .dao_rules import (
     schedule_rule_to_storage,
 )
 from .db import NestQuestDatabase
-from .recurrence import ScheduleRule, occurs_on
+from .recurrence import ScheduleRule
 
 
 @dataclass(frozen=True)
 class CreatedQuestDefinition:
-    """The persisted result of a create, edit or activation change.
+    """The persisted result of a create or edit.
 
     ``definition`` is the stored row; ``rule`` is the schedule rule
     decoded back through the storage mapping; ``assignees`` and
@@ -319,23 +315,14 @@ async def edit_quest_definition(
         window_specs = _normalize_windows(windows)
 
     dao = QuestDefinitionsDao(database)
-    try:
+    snapshot = await dao.edit_definition(
-        snapshot = await dao.edit_definition(
+        definition_id,
-            definition_id,
+        title=title_value,
-            title=title_value,
+        description=description_value,
-            description=description_value,
+        icon=icon_value,
-            icon=icon_value,
+        rule=rule_storage,
-            rule=rule_storage,
+        windows=window_specs,
-            windows=window_specs,
+    )
-        )
-    except ValueError as error:
-        # The DAO's missing-definition error names only the id; re-raise
-        # so the public contract names the field the caller passed (matching
-        # assign/unassign/deactivate).
-        message = str(error)
-        if message.startswith("quest definition "):
-            raise ValueError(f"definition_id: {message}") from error
-        raise
 
     decoded_rule = schedule_rule_from_storage(
         schedule_rule_storage_from_record(snapshot.rule)
@@ -406,146 +393,3 @@ async def unassign_child(
             f"definition_id: quest definition {definition_id} does not exist"
         )
     return await dao.remove_assignee(definition_id, child_id)
-
-
-async def set_quest_definition_active(
-    database: NestQuestDatabase,
-    definition_id: int,
-    is_active: bool,
-) -> CreatedQuestDefinition:
-    """Deactivate or reactivate a definition; returns the updated view.
-
-    Flipping the flag is the only removal path (Feature 06 guardrail):
-    a deactivated definition stops future instance generation but its
-    row, schedule rule, assignees and windows all survive, and existing
-    instances plus completion history are never touched.  Reactivation
-    re-enables future generation; generating or removing instances is
-    Feature 07 and does not happen here.
-
-    ``definition_id`` must be a plain int (bools and floats rejected,
-    since SQLite would bind ``True`` onto definition 1) and
-    ``is_active`` must be a real bool — the DAO's ``int()`` would
-    otherwise silently coerce strings and numerics ("1", 0) into a
-    state the caller never asked for.  Raises ValueError naming the
-    offending field when the id is malformed, ``is_active`` is not a
-    bool, or the definition does not exist.
-
-    Atomicity: the existence check, the UPDATE and the readback of the
-    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.
-    """
-    _validate_definition_id(definition_id)
-    if not isinstance(is_active, bool):
-        raise ValueError(f"is_active must be a real bool, got {is_active!r}")
-    async with _connection_lock(database):
-        dao = QuestDefinitionsDao(database)
-        if await dao.get(definition_id) is None:
-            raise ValueError(
-                f"definition_id: quest definition {definition_id} "
-                "does not exist"
-            )
-        await dao.set_active(definition_id, is_active)
-        updated = await dao.get(definition_id)
-        assert updated is not None
-        rule_record = await ScheduleRulesDao(database).get(
-            updated.schedule_rule_id
-        )
-        assert rule_record is not None
-        assignees = await dao.list_assignees(definition_id)
-        windows = await dao.list_windows(definition_id)
-    rule = schedule_rule_from_storage(
-        schedule_rule_storage_from_record(rule_record)
-    )
-    return CreatedQuestDefinition(
-        definition=updated,
-        rule=rule,
-        assignees=assignees,
-        windows=windows,
-    )
-
-
-def _bundle_snapshot(
-    snapshot: QuestDefinitionSnapshot,
-) -> CreatedQuestDefinition:
-    """Decode a DAO snapshot into the layer's rich view.
-
-    The rule record carried by the snapshot is the one the definition
-    referenced at read time, so decoding it here (rather than re-reading
-    through :class:`~.dao_rules.ScheduleRulesDao`) preserves the
-    coherence the DAO's locked snapshot guarantees: the rule, assignees
-    and windows all come from one atomic read, never three separate ones.
-    """
-    rule = schedule_rule_from_storage(
-        schedule_rule_storage_from_record(snapshot.rule)
-    )
-    return CreatedQuestDefinition(
-        definition=snapshot.definition,
-        rule=rule,
-        assignees=snapshot.assignees,
-        windows=snapshot.windows,
-    )
-
-
-async def list_active_definitions(
-    database: NestQuestDatabase,
-) -> list[CreatedQuestDefinition]:
-    """Return every ACTIVE definition with its decoded rule and links.
-
-    Ordered by rising definition id, matching the DAO's ``list_active``
-    order, so callers get a stable, repeatable sequence.  Each entry is
-    the same rich :class:`CreatedQuestDefinition` snapshot as the rest of
-    the layer: the stored row plus its decoded
-    :class:`~.recurrence.ScheduleRule`, assignees and windows.  The
-    definitions and their links are read by the DAO inside one locked
-    transaction, so each bundle is a coherent snapshot that a concurrent
-    edit cannot interleave.
-    """
-    dao = QuestDefinitionsDao(database)
-    snapshots = await dao.list_snapshots_active()
-    return [_bundle_snapshot(snapshot) for snapshot in snapshots]
-
-
-async def list_definitions_for_child(
-    database: NestQuestDatabase,
-    child_id: int,
-) -> list[CreatedQuestDefinition]:
-    """Return the definitions assigned to ``child_id`` (rich view).
-
-    ``child_id`` must be a plain int (bools and floats rejected, since
-    SQLite would bind ``True`` onto child 1).  Results are ordered by
-    rising definition id, matching ``list_active_definitions`` and
-    ``list_definitions_firing_on``.  Each entry is a
-    :class:`CreatedQuestDefinition` snapshot bundling the definition with
-    its decoded rule, assignees and windows, read atomically by the DAO.
-    """
-    _validate_child_id(child_id)
-    dao = QuestDefinitionsDao(database)
-    snapshots = await dao.list_snapshots_by_child(child_id)
-    bundles = [_bundle_snapshot(snapshot) for snapshot in snapshots]
-    return sorted(bundles, key=lambda bundle: bundle.definition.id)
-
-
-async def list_definitions_firing_on(
-    database: NestQuestDatabase,
-    date: str,
-) -> list[CreatedQuestDefinition]:
-    """Return the ACTIVE definitions whose rule fires on ``date``.
-
-    ``date`` must be a strict ISO calendar date (YYYY-MM-DD); a malformed
-    value raises ValueError naming the field.  Each active definition's
-    stored rule is decoded through the task-1 storage mapping and handed
-    to the recurrence engine's :func:`~.recurrence.occurs_on`; only the
-    definitions whose decoded rule fires on the date are returned, in
-    rising definition-id order, each as the same rich
-    :class:`CreatedQuestDefinition` snapshot as the rest of the layer.
-    The definitions and their rules are read inside one locked
-    transaction, so a bundle can never filter on one rule while
-    presenting another state's windows.
-    """
-    _validate_date(date, "date")
-    target = datetime.datetime.strptime(date, "%Y-%m-%d").date()
-    dao = QuestDefinitionsDao(database)
-    snapshots = await dao.list_snapshots_active()
-    bundles = [_bundle_snapshot(snapshot) for snapshot in snapshots]
-    return [bundle for bundle in bundles if occurs_on(bundle.rule, target)]

tests/test_quest_definitions.py

@@ -2,16 +2,11 @@
 from __future__ import annotations
 
 import asyncio
-import datetime
 import re
 
 import pytest
 
 from custom_components.nestquest.dao_children import ChildrenDao
-from custom_components.nestquest.dao_instances import (
-    CompletionEventsDao,
-    QuestInstancesDao,
-)
 from custom_components.nestquest.dao_rules import (
     QuestDefinitionsDao,
     ScheduleRulesDao,
@@ -24,10 +19,6 @@ from custom_components.nestquest.quest_definitions import (
     assign_child,
     create_quest_definition,
     edit_quest_definition,
-    list_active_definitions,
-    list_definitions_firing_on,
-    list_definitions_for_child,
-    set_quest_definition_active,
     unassign_child,
 )
 from custom_components.nestquest.recurrence import RuleType, ScheduleRule
@@ -81,15 +72,6 @@ def _daily_rule() -> ScheduleRule:
     return ScheduleRule(rule_type=RuleType.DAILY, start_date="2026-09-14")
 
 
-def _today_iso() -> str:
-    """Today's date in ISO form, computed at call time (never cached).
-
-    ``QuestInstancesDao.upsert`` refuses past due dates, so seeded
-    instances use a date that is always today-or-later at test time.
-    """
-    return datetime.date.today().isoformat()
-
-
 # ---------------------------------------------------------------------------
 # create_quest_definition: successful creates
 # ---------------------------------------------------------------------------
@@ -786,24 +768,6 @@ def test_edit_rejects_nonexistent_definition(tmp_path) -> None:
     _with_db(tmp_path, "edit-missing-definition.db")(_body)
 
 
-def test_edit_nonexistent_definition_names_field(tmp_path) -> None:
-    """A missing definition's error names the ``definition_id`` field.
-
-    The DAO raises a bare "quest definition <id> does not exist"; the
-    business layer must re-raise it prefixed with the field the caller
-    passed, matching assign/unassign/deactivate.
-    """
-    async def _body(database, children, rules, definitions, child):
-        with pytest.raises(
-            ValueError,
-            match="definition_id: quest definition 999 does not exist",
-        ):
-            await edit_quest_definition(database, 999, title="X")
-        return None
-
-    _with_db(tmp_path, "edit-missing-definition-field.db")(_body)
-
-
 @pytest.mark.parametrize("bad", [True, 1.5, "1", None])
 def test_edit_rejects_non_int_definition_id(tmp_path, bad) -> None:
     async def _body(database, children, rules, definitions, child):
@@ -1186,761 +1150,3 @@ def test_concurrent_assign_unassign_returns_consistent_roster(
         return assignees
 
     _with_db(tmp_path, "assign-unassign-concurrent.db")(_body)
-
-
-# ---------------------------------------------------------------------------
-# set_quest_definition_active: deactivate / reactivate
-# ---------------------------------------------------------------------------
-
-
-def test_set_quest_definition_active_round_trip(tmp_path) -> None:
-    async def _body(database, children, rules, definitions, child):
-        created = await create_quest_definition(
-            database, "Brush teeth", _daily_rule(), [child.id], ["morning"]
-        )
-        definition_id = created.definition.id
-
-        deactivated = await set_quest_definition_active(
-            database, definition_id, False
-        )
-        assert isinstance(deactivated, CreatedQuestDefinition)
-        assert deactivated.definition.id == definition_id
-        assert deactivated.definition.is_active is False
-        # The decoded rule, assignees and windows form a consistent
-        # snapshot of the (unchanged) definition.
-        assert deactivated.rule == _daily_rule()
-        assert [c.id for c in deactivated.assignees] == [child.id]
-        assert [w.window for w in deactivated.windows] == ["morning"]
-        assert await definitions.list_active() == []
-
-        reactivated = await set_quest_definition_active(
-            database, definition_id, True
-        )
-        assert reactivated.definition.is_active is True
-        assert [d.id for d in await definitions.list_active()] == [
-            definition_id
-        ]
-        return reactivated
-
-    _with_db(tmp_path, "set-active-round-trip.db")(_body)
-
-
-@pytest.mark.parametrize("bad", ["1", "0", 1, 0, 1.0, None, "true"])
-def test_set_quest_definition_active_rejects_non_bool(tmp_path, bad) -> None:
-    async def _body(database, children, rules, definitions, child):
-        created = await create_quest_definition(
-            database, "Brush teeth", _daily_rule(), [child.id], ["morning"]
-        )
-        with pytest.raises(
-            ValueError, match="is_active must be a real bool"
-        ):
-            await set_quest_definition_active(
-                database, created.definition.id, bad
-            )
-        # The refused transition left the definition active.
-        stored = await definitions.get(created.definition.id)
-        assert stored is not None
-        assert stored.is_active is True
-        return None
-
-    _with_db(tmp_path, "set-active-non-bool.db")(_body)
-
-
-@pytest.mark.parametrize("bad", [True, False, 1.5, "1", None])
-def test_set_quest_definition_active_rejects_non_int_definition_id(
-    tmp_path, bad
-) -> None:
-    async def _body(database, children, rules, definitions, child):
-        created = await create_quest_definition(
-            database, "Brush teeth", _daily_rule(), [child.id], ["morning"]
-        )
-        with pytest.raises(
-            ValueError, match="definition_id must be an integer"
-        ):
-            await set_quest_definition_active(database, bad, False)
-        stored = await definitions.get(created.definition.id)
-        assert stored is not None
-        assert stored.is_active is True
-        return None
-
-    _with_db(tmp_path, "set-active-non-int-id.db")(_body)
-
-
-def test_set_quest_definition_active_nonexistent_definition_names_field(
-    tmp_path,
-) -> None:
-    async def _body(database, children, rules, definitions, child):
-        with pytest.raises(
-            ValueError, match="definition_id.*does not exist"
-        ):
-            await set_quest_definition_active(database, 999, False)
-        return None
-
-    _with_db(tmp_path, "set-active-missing-definition.db")(_body)
-
-
-def test_set_quest_definition_active_never_deletes_or_touches_instances(
-    tmp_path,
-) -> None:
-    """Deactivation hides the definition but deletes nothing, and
-    pre-existing instances and completion history survive unchanged."""
-    async def _body(database, children, rules, definitions, child):
-        created = await create_quest_definition(
-            database, "Brush teeth", _daily_rule(), [child.id], ["morning"]
-        )
-        definition_id = created.definition.id
-        rule_id = created.definition.schedule_rule_id
-
-        # Seed a real instance and a real completion event against the
-        # definition BEFORE deactivation, so the test can detect a
-        # delete or mutation of pre-existing data (not just an empty
-        # table staying empty).
-        instances = QuestInstancesDao(database)
-        events = CompletionEventsDao(database)
-        instance = await instances.upsert(
-            definition_id, child.id, _today_iso(), NOW, window="morning"
-        )
-        event = await events.append(
-            instance.id,
-            child.id,
-            "completed",
-            "user",
-            NOW,
-            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"
-        )
-
-        await set_quest_definition_active(database, definition_id, False)
-
-        # The definition row survives (deactivated, not deleted).
-        stored = await definitions.get(definition_id)
-        assert stored is not None
-        assert stored.is_active is False
-        # Its rule, assignee and window rows all survive too.
-        assert await rules.get(rule_id) is not None
-        assert [c.id for c in await definitions.list_assignees(
-            definition_id
-        )] == [child.id]
-        assert [w.window for w in await definitions.list_windows(
-            definition_id
-        )] == ["morning"]
-        # The pre-existing instance and completion event survive with
-        # 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
-        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)
-
-
-def _gated_hass(
-    armed: dict, gate_open: asyncio.Event, a_started: asyncio.Event
-):
-    """A hass mock whose executor gates the FIRST ``_execute`` job after
-    arming — the UPDATE of whichever ``set_quest_definition_active``
-    call starts first — proving the caller holds the connection lock at
-    that moment and the second caller must queue behind it."""
-    from unittest.mock import MagicMock
-
-    hass = MagicMock()
-
-    async def _gated_executor(fn, *args):
-        if armed["active"]:
-            if getattr(fn, "__name__", "") == "_execute" and not armed["gated"]:
-                armed["gated"] = True
-                a_started.set()
-                await gate_open.wait()
-        return fn(*args)
-
-    hass.async_add_executor_job = _gated_executor
-    return hass
-
-
-def test_set_quest_definition_active_concurrent_opposite_transitions_return_own(
-    tmp_path,
-) -> None:
-    """Racing deactivate/reactivate calls each report their own request.
-
-    Deterministic gate on task A's UPDATE: task B queues behind the
-    connection lock and lands the opposite transition after A.  Without
-    the lock, A's readback could see B's write and report B's value.
-    """
-    async def _main():
-        armed = {"active": False, "gated": False}
-        gate_open = asyncio.Event()
-        a_started = asyncio.Event()
-        hass = _gated_hass(armed, gate_open, a_started)
-        database = NestQuestDatabase(hass)
-        await database.open(tmp_path / "active-race.db")
-        try:
-            await apply_migrations(database)
-            children = ChildrenDao(database)
-            child = await children.create("Ada", NOW)
-            created = await create_quest_definition(
-                database, "Brush teeth", _daily_rule(), [child.id], ["morning"]
-            )
-
-            armed["active"] = True
-            task_a = asyncio.ensure_future(
-                set_quest_definition_active(
-                    database, created.definition.id, False
-                )
-            )
-            await a_started.wait()
-            armed["active"] = False
-            task_b = asyncio.ensure_future(
-                set_quest_definition_active(
-                    database, created.definition.id, True
-                )
-            )
-            await asyncio.sleep(0)
-            gate_open.set()
-            record_a, record_b = await asyncio.gather(task_a, task_b)
-
-            # Each caller gets back exactly the state it requested.
-            assert record_a.definition.is_active is False
-            assert record_b.definition.is_active is True
-            # Both snapshots stay coherent: the decoded rule and the
-            # assignees are those of the definition they were read from.
-            assert record_a.rule == _daily_rule()
-            assert record_b.rule == _daily_rule()
-            assert [c.id for c in record_a.assignees] == [child.id]
-            assert [c.id for c in record_b.assignees] == [child.id]
-            # B landed last: the stored row is B's (read via the DAO —
-            # the four-table leak guard forbids raw probes here).
-            stored = await QuestDefinitionsDao(database).get(
-                created.definition.id
-            )
-            assert stored is not None
-            assert stored.is_active is True
-        finally:
-            await database.close()
-
-    _run(_main())
-
-
-# ---------------------------------------------------------------------------
-# list query helpers: list_active_definitions / list_definitions_for_child /
-# list_definitions_firing_on
-# ---------------------------------------------------------------------------
-
-
-def test_list_active_definitions_returns_only_active_in_id_order(tmp_path) -> None:
-    """Only active definitions come back, in rising id order, each rich.
-
-    Two definitions are created (ids 1 and 2), then definition 1 is
-    deactivated.  ``list_active_definitions`` must return only definition
-    2, and every bundle must carry the decoded rule, assignees and
-    windows of its own definition.
-    """
-    async def _body(database, children, rules, definitions, child):
-        bo = await children.create("Bo", NOW)
-        first = await create_quest_definition(
-            database, "Brush teeth", _daily_rule(), [child.id], ["morning"]
-        )
-        weekly = ScheduleRule(
-            rule_type=RuleType.WEEKLY,
-            weekday_set={0, 2},
-            start_date="2026-09-14",
-        )
-        second = await create_quest_definition(
-            database,
-            "Tidy room",
-            weekly,
-            [child.id, bo.id],
-            [("morning", "08:00"), ("evening", "19:00")],
-        )
-        assert [d.id for d in await definitions.list_active()] == [
-            first.definition.id,
-            second.definition.id,
-        ]
-
-        await set_quest_definition_active(database, first.definition.id, False)
-
-        result = await list_active_definitions(database)
-        assert [bundle.definition.id for bundle in result] == [second.definition.id]
-        assert isinstance(result[0], CreatedQuestDefinition)
-        assert result[0].rule == weekly
-        assert sorted(c.id for c in result[0].assignees) == sorted(
-            [child.id, bo.id]
-        )
-        by_name = {w.window: w.due_time for w in result[0].windows}
-        assert by_name == {"morning": "08:00", "evening": "19:00"}
-        return result
-
-    _with_db(tmp_path, "list-active.db")(_body)
-
-
-def test_list_definitions_for_child_returns_only_that_child(tmp_path) -> None:
-    """Only the given child's definitions come back, deterministically.
-
-    Definitions are created assigned to Ada (child), Bo, or both; the
-    helper must return exactly the definitions assigned to Ada, excluding
-    a Bo-only definition.  Inactive definitions are still assigned, so
-    they too come back (the helper filters by child, not by activity).
-    """
-    async def _body(database, children, rules, definitions, child):
-        bo = await children.create("Bo", NOW)
-        ada_only = await create_quest_definition(
-            database, "Brush teeth", _daily_rule(), [child.id], ["morning"]
-        )
-        both = await create_quest_definition(
-            database,
-            "Tidy room",
-            ScheduleRule(
-                rule_type=RuleType.WEEKLY,
-                weekday_set={1},
-                start_date="2026-09-14",
-            ),
-            [child.id, bo.id],
-            ["evening"],
-        )
-        bo_only = await create_quest_definition(
-            database, "Walk dog", _daily_rule(), [bo.id], ["afternoon"]
-        )
-
-        # The helper returns the given child's definitions in ascending
-        # id order, consistent with list_active_definitions.
-        result = await list_definitions_for_child(database, child.id)
-        assert [bundle.definition.id for bundle in result] == [
-            ada_only.definition.id,
-            both.definition.id,
-        ]
-        assert result[0].rule == _daily_rule()
-        assert [c.id for c in result[0].assignees] == [child.id]
-        assert [w.window for w in result[0].windows] == ["morning"]
-
-        bo_result = await list_definitions_for_child(database, bo.id)
-        assert [bundle.definition.id for bundle in bo_result] == [
-            both.definition.id,
-            bo_only.definition.id,
-        ]
-        return result
-
-    _with_db(tmp_path, "list-for-child.db")(_body)
-
-
-@pytest.mark.parametrize("bad", [True, False, 1.5, "1", None])
-def test_list_definitions_for_child_rejects_non_int_child_id(tmp_path, bad) -> None:
-    async def _body(database, children, rules, definitions, child):
-        with pytest.raises(ValueError, match="child_id must be an integer"):
-            await list_definitions_for_child(database, bad)
-        return None
-
-    _with_db(tmp_path, "list-for-child-bad-id.db")(_body)
-
-
-def test_list_definitions_firing_on_mixed_rules(tmp_path) -> None:
-    """A mixed daily + weekly + monthly set fires on the right dates.
-
-    Daily fires every day from its start; weekly fires on its weekdays;
-    monthly fires on its day-of-month.  Deactivated definitions never
-    fire, and the returned bundles carry the decoded rule.
-    """
-    async def _body(database, children, rules, definitions, child):
-        daily = await create_quest_definition(
-            database,
-            "Daily chore",
-            ScheduleRule(rule_type=RuleType.DAILY, start_date="2026-09-14"),
-            [child.id],
-            ["morning"],
-        )
-        weekly = await create_quest_definition(
-            database,
-            "Weekly chore",
-            ScheduleRule(
-                rule_type=RuleType.WEEKLY,
-                weekday_set={0, 2},  # Monday, Wednesday
-                start_date="2026-09-14",
-            ),
-            [child.id],
-            ["evening"],
-        )
-        monthly = await create_quest_definition(
-            database,
-            "Monthly chore",
-            ScheduleRule(
-                rule_type=RuleType.MONTHLY_DAY,
-                day_of_month=15,
-                start_date="2026-09-14",
-            ),
-            [child.id],
-            ["afternoon"],
-        )
-
-        # 2026-09-14 is a Monday (weekday 0): daily + weekly fire;
-        # monthly (the 15th) does not.
-        hits = await list_definitions_firing_on(database, "2026-09-14")
-        assert [b.definition.id for b in hits] == [
-            daily.definition.id,
-            weekly.definition.id,
-        ]
-        # 2026-09-15 is a Tuesday (weekday 1, day 15): daily + monthly.
-        hits = await list_definitions_firing_on(database, "2026-09-15")
-        assert [b.definition.id for b in hits] == [
-            daily.definition.id,
-            monthly.definition.id,
-        ]
-        # 2026-09-16 is a Wednesday (weekday 2, day 16): daily + weekly.
-        hits = await list_definitions_firing_on(database, "2026-09-16")
-        assert [b.definition.id for b in hits] == [
-            daily.definition.id,
-            weekly.definition.id,
-        ]
-
-        # Deactivating a definition removes it from every future result.
-        await set_quest_definition_active(database, weekly.definition.id, False)
-        hits = await list_definitions_firing_on(database, "2026-09-14")
-        assert [b.definition.id for b in hits] == [daily.definition.id]
-        # The returned bundle carries the decoded rule, not the raw row.
-        assert hits[0].rule == ScheduleRule(
-            rule_type=RuleType.DAILY, start_date="2026-09-14"
-        )
-        return hits
-
-    _with_db(tmp_path, "list-firing-on.db")(_body)
-
-
-@pytest.mark.parametrize(
-    "bad",
-    ["2026-9-4", "2026-13-01", "2026-02-30", "not-a-date", None, 20260914],
-)
-def test_list_definitions_firing_on_rejects_malformed_date(tmp_path, bad) -> None:
-    async def _body(database, children, rules, definitions, child):
-        await create_quest_definition(
-            database, "Brush teeth", _daily_rule(), [child.id], ["morning"]
-        )
-        with pytest.raises(ValueError, match="date must be"):
-            await list_definitions_firing_on(database, bad)
-        return None
-
-    _with_db(tmp_path, "list-firing-on-bad-date.db")(_body)
-
-
-def test_list_query_snapshot_coherent_under_racing_edit(tmp_path) -> None:
-    """A racing edit cannot mix states into a query helper's snapshot.
-
-    The query helper pauses mid-snapshot (inside its locked transaction)
-    while an edit that changes BOTH the rule and the windows is queued
-    behind the connection lock.  Because the helper reads definitions,
-    rules and windows in one locked transaction, its returned bundle is
-    either the whole old state or the whole new state — never the old
-    rule paired with the new windows (the exact interleaving the unlocked
-    reads used to allow).  The pause hook holds
-    ``list_windows`` mid-flight so the edit is deterministically queued
-    against the helper's lock.
-    """
-    async def _body(database, children, rules, definitions, child):
-        created = await create_quest_definition(
-            database, "Brush teeth", _daily_rule(), [child.id], ["morning"]
-        )
-        weekly = ScheduleRule(
-            rule_type=RuleType.WEEKLY,
-            weekday_set={2},
-            start_date="2026-09-14",
-        )
-
-        import custom_components.nestquest.dao_rules as dao_rules
-
-        original_list = dao_rules.QuestDefinitionsDao.list_windows
-        started = asyncio.Event()
-        release = asyncio.Event()
-
-        async def _pausing_list(self, definition_id):
-            if not started.is_set():
-                started.set()
-                await release.wait()
-            return await original_list(self, definition_id)
-
-        dao_rules.QuestDefinitionsDao.list_windows = _pausing_list
-        try:
-            read_task = asyncio.ensure_future(
-                list_definitions_firing_on(database, "2026-09-16")
-            )
-            await started.wait()
-            edit_task = asyncio.ensure_future(
-                edit_quest_definition(
-                    database,
-                    created.definition.id,
-                    rule=weekly,
-                    windows=["evening"],
-                )
-            )
-            await asyncio.sleep(0)
-            release.set()
-            result, edited = await asyncio.gather(read_task, edit_task)
-        finally:
-            dao_rules.QuestDefinitionsDao.list_windows = original_list
-
-        # The helper finished before the queued edit, so its snapshot is
-        # the whole OLD state: the daily rule with its morning window.
-        assert [b.definition.id for b in result] == [created.definition.id]
-        assert result[0].rule == _daily_rule()
-        assert [w.window for w in result[0].windows] == ["morning"]
-        # The edit then reported the whole NEW state, not a mix either.
-        assert edited.rule == weekly
-        assert [w.window for w in edited.windows] == ["evening"]
-        return result
-
-    _with_db(tmp_path, "list-snapshot-concurrent.db")(_body)
-
-
-def test_list_definitions_for_child_snapshot_coherent_under_racing_unassign(
-    tmp_path,
-) -> None:
-    """A racing unassign cannot strip the child from the child-scoped
-    snapshot that ``list_definitions_for_child`` returns.
-
-    The helper pauses mid-snapshot (inside its locked transaction) while
-    an unassign of the very child it is listing is queued behind the
-    connection lock.  Because the definition set and its assignees are
-    read in one locked transaction, the returned bundle still shows the
-    child assigned — the coherent pre-unassign snapshot — rather than the
-    mixed state (definition returned "for child" but with the child
-    already gone from its roster) that the unlocked reads used to allow.
-    The pause hook holds ``list_assignees`` mid-flight so the unassign is
-    deterministically queued against the helper's lock.
-    """
-    async def _body(database, children, rules, definitions, child):
-        created = await create_quest_definition(
-            database, "Brush teeth", _daily_rule(), [child.id], ["morning"]
-        )
-
-        import custom_components.nestquest.dao_rules as dao_rules
-
-        original_list = dao_rules.QuestDefinitionsDao.list_assignees
-        started = asyncio.Event()
-        release = asyncio.Event()
-
-        async def _pausing_list(self, definition_id):
-            if not started.is_set():
-                started.set()
-                await release.wait()
-            return await original_list(self, definition_id)
-
-        dao_rules.QuestDefinitionsDao.list_assignees = _pausing_list
-        try:
-            read_task = asyncio.ensure_future(
-                list_definitions_for_child(database, child.id)
-            )
-            await started.wait()
-            unassign_task = asyncio.ensure_future(
-                unassign_child(database, created.definition.id, child.id)
-            )
-            for _ in range(5):
-                await asyncio.sleep(0)
-            release.set()
-            result, removed = await asyncio.gather(read_task, unassign_task)
-        finally:
-            dao_rules.QuestDefinitionsDao.list_assignees = original_list
-
-        # The helper finished before the queued unassign, so its snapshot
-        # is the whole OLD state: the child still assigned to the
-        # definition it returned for that child.
-        assert [b.definition.id for b in result] == [created.definition.id]
-        assert [c.id for c in result[0].assignees] == [child.id]
-        assert removed is True
-        return result
-
-    _with_db(tmp_path, "list-for-child-snapshot-concurrent.db")(_body)
-
-
-def test_list_active_definitions_snapshot_coherent_under_racing_edit(
-    tmp_path,
-) -> None:
-    """A racing edit cannot mix states into ``list_active_definitions``.
-
-    The helper pauses mid-snapshot (inside its locked transaction) while
-    an edit that changes BOTH the rule and the windows is queued behind
-    the connection lock.  Because definitions, rules and windows are read
-    in one locked transaction, the returned bundle is the whole old state
-    — the daily rule with its morning window — never the old rule paired
-    with the new windows.  The pause hook holds ``list_windows``
-    mid-flight so the edit is deterministically queued against the
-    helper's lock.
-    """
-    async def _body(database, children, rules, definitions, child):
-        created = await create_quest_definition(
-            database, "Brush teeth", _daily_rule(), [child.id], ["morning"]
-        )
-        weekly = ScheduleRule(
-            rule_type=RuleType.WEEKLY,
-            weekday_set={2},
-            start_date="2026-09-14",
-        )
-
-        import custom_components.nestquest.dao_rules as dao_rules
-
-        original_list = dao_rules.QuestDefinitionsDao.list_windows
-        started = asyncio.Event()
-        release = asyncio.Event()
-
-        async def _pausing_list(self, definition_id):
-            if not started.is_set():
-                started.set()
-                await release.wait()
-            return await original_list(self, definition_id)
-
-        dao_rules.QuestDefinitionsDao.list_windows = _pausing_list
-        try:
-            read_task = asyncio.ensure_future(list_active_definitions(database))
-            await started.wait()
-            edit_task = asyncio.ensure_future(
-                edit_quest_definition(
-                    database,
-                    created.definition.id,
-                    rule=weekly,
-                    windows=["evening"],
-                )
-            )
-            await asyncio.sleep(0)
-            release.set()
-            result, edited = await asyncio.gather(read_task, edit_task)
-        finally:
-            dao_rules.QuestDefinitionsDao.list_windows = original_list
-
-        # The helper finished before the queued edit, so its snapshot is
-        # the whole OLD state: the daily rule with its morning window.
-        assert [b.definition.id for b in result] == [created.definition.id]
-        assert result[0].rule == _daily_rule()
-        assert [w.window for w in result[0].windows] == ["morning"]
-        # The edit then reported the whole NEW state, not a mix either.
-        assert edited.rule == weekly
-        assert [w.window for w in edited.windows] == ["evening"]
-        return result
-
-    _with_db(tmp_path, "list-active-snapshot-concurrent.db")(_body)
-
-
-# ---------------------------------------------------------------------------
-# full-lifecycle integration: one definition through every operation
-# ---------------------------------------------------------------------------
-
-
-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.
-
-    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.
-    """
-    async def _body(database, children, rules, definitions, child):
-        created = await create_quest_definition(
-            database,
-            "Brush teeth",
-            _daily_rule(),
-            [child.id],
-            [("morning", "07:00")],
-        )
-        definition_id = created.definition.id
-
-        instances = QuestInstancesDao(database)
-        events = CompletionEventsDao(database)
-        instance = await instances.upsert(
-            definition_id, child.id, _today_iso(), NOW, window="morning"
-        )
-        event = await events.append(
-            instance.id,
-            child.id,
-            "completed",
-            "user",
-            NOW,
-            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():
-            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()
-
-        # Edit rule and windows together; the definition's rule row is
-        # rewritten in place and its window set replaced.
-        edited = await edit_quest_definition(
-            database,
-            definition_id,
-            rule=ScheduleRule(
-                rule_type=RuleType.WEEKLY,
-                weekday_set={1, 3},
-                start_date="2026-09-14",
-            ),
-            windows=[("morning", "08:00"), ("evening", "19:30")],
-        )
-        assert edited.rule == ScheduleRule(
-            rule_type=RuleType.WEEKLY,
-            weekday_set={1, 3},
-            start_date="2026-09-14",
-        )
-        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()
-
-        # 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()
-
-        # 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()
-
-        # 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()
-        reactivated = await set_quest_definition_active(
-            database, definition_id, True
-        )
-        assert reactivated.definition.is_active is True
-        await _assert_history_untouched()
-
-        return reactivated
-
-    _with_db(tmp_path, "full-lifecycle.db")(_body)