feat: derive instance state from latest completion event

custom_components/nestquest/completion.py

@@ -4,11 +4,11 @@ Sits between the Feature 09 service gate and the typed
 :class:`~.dao_instances.CompletionEventsDao`: callers get actor-shape
 validation and the UTC occurred-at stamp here, and never talk to the
 DAO or SQL directly.  This module is the only Feature-08 caller of
-:meth:`~.dao_instances.CompletionEventsDao.append`.  Completing an
+:meth:`~.dao_instances.CompletionEventsDao.append`.  Completing and
-instance (no-op if already done) is :func:`complete_instance`;
+un-completing an instance (no-op-if-already-done) arrive in later
-un-completing arrives in a later task.  Instance current state is
+tasks.  Instance current state is derived from the latest event via
-derived from the latest event via :func:`instance_state`
+:func:`instance_state` (``open`` / ``done``); there is no status
-(``open`` / ``done``); there is no status column.
+column.
 
 Actor policy matches the schema CHECKs (D-008): ``actor_source`` is
 ``'user'`` or ``'panel'`` only — never ``'service'``.  ``'user'``
@@ -25,10 +25,9 @@ is no permission check here (that is Feature 09).
 """
 from __future__ import annotations
 
-import asyncio
 import datetime
 
-from .dao_children import _CONNECTION_LOCKS, _connection_lock
+from .dao_children import _connection_lock
 from .dao_instances import (
     EVENT_COMPLETED,
     EVENT_UNCOMPLETED,
@@ -63,63 +62,6 @@ def _stamp_occurred_at(now: datetime.datetime | None) -> str:
     )
 
 
-class _TaskReentrantLock:
-    """Shared connection lock the owning task may re-enter."""
-
-    def __init__(self, inner: asyncio.Lock) -> None:
-        self._inner = inner
-        self._task: asyncio.Task | None = None
-        self._depth = 0
-
-    def locked(self) -> bool:
-        return self._inner.locked()
-
-    async def acquire(self) -> bool:
-        task = asyncio.current_task()
-        if self._task is task:
-            self._depth += 1
-            return True
-        await self._inner.acquire()
-        self._task = task
-        self._depth = 1
-        return True
-
-    def release(self) -> None:
-        if self._task is not asyncio.current_task():
-            raise RuntimeError("lock released by a non-owner")
-        self._depth -= 1
-        if self._depth == 0:
-            self._task = None
-            self._inner.release()
-
-    async def __aenter__(self) -> _TaskReentrantLock:
-        await self.acquire()
-        return self
-
-    async def __aexit__(self, *exc: object) -> None:
-        self.release()
-
-
-def _reentrant_connection_lock(database) -> _TaskReentrantLock:
-    """Return the shared connection lock, made task-reentrant.
-
-    ``CompletionEventsDao.append`` also acquires this lock, so
-    check+append in :func:`complete_instance` must re-enter on the
-    same task.  The wrapper is stored in the shared dict so the DAO
-    sees the same object.
-    """
-    loop = asyncio.get_running_loop()
-    key = (id(database), id(loop))
-    lock = _CONNECTION_LOCKS.get(key)
-    if isinstance(lock, _TaskReentrantLock):
-        return lock
-    wrapped = _TaskReentrantLock(
-        lock if lock is not None else asyncio.Lock()
-    )
-    _CONNECTION_LOCKS[key] = wrapped
-    return wrapped
-
-
 def _validate_int_id(value: object, field: str) -> int:
     """Reject non-int ids (bools included) before any lookup."""
     if isinstance(value, bool) or not isinstance(value, int):
@@ -256,61 +198,3 @@ async def instance_state(
         f"event_type must be '{EVENT_COMPLETED}' or "
         f"'{EVENT_UNCOMPLETED}', got {latest.event_type!r}"
     )
-
-
-async def complete_instance(
-    database: NestQuestDatabase,
-    instance_id: int,
-    *,
-    actor_source: str,
-    actor_user_id: str | None = None,
-    actor_child_id: int | None = None,
-    now: datetime.datetime | None = None,
-    today: datetime.date | None = None,
-) -> str:
-    """Append a completed event unless the instance is already done.
-
-    Returns the derived state ``done``.  An already-done instance is a
-    no-op: nothing is appended.  Unknown ``instance_id`` raises
-    ValueError naming the field.  Existence, latest-event, and the
-    append run under one connection lock so a concurrent complete
-    cannot double-append.
-
-    ``was_on_time`` is recorded as True — a placeholder until the
-    on-time task computes it from ``now`` / ``today`` against the
-    instance due_date and optional due_time.  ``today`` is accepted so
-    callers can thread the HA-local date through without a signature
-    change; it is unused here.
-    """
-    instance_id = _validate_int_id(instance_id, "instance_id")
-    actor_source, actor_user_id, actor_child_id = _validate_actor(
-        actor_source, actor_user_id, actor_child_id
-    )
-    async with _reentrant_connection_lock(database):
-        instance = await QuestInstancesDao(database).get_by_id(instance_id)
-        if instance is None:
-            raise ValueError(
-                f"instance_id: quest instance {instance_id} does not exist"
-            )
-        latest = await CompletionEventsDao(database).get_latest_for_instance(
-            instance_id
-        )
-        if latest is not None and latest.event_type == EVENT_COMPLETED:
-            return "done"
-        if latest is not None and latest.event_type != EVENT_UNCOMPLETED:
-            raise ValueError(
-                f"event_type must be '{EVENT_COMPLETED}' or "
-                f"'{EVENT_UNCOMPLETED}', got {latest.event_type!r}"
-            )
-        await append_event(
-            database,
-            instance.id,
-            instance.child_id,
-            EVENT_COMPLETED,
-            actor_source=actor_source,
-            actor_user_id=actor_user_id,
-            actor_child_id=actor_child_id,
-            was_on_time=True,
-            now=now,
-        )
-    return "done"

tests/test_completion.py

@@ -8,11 +8,7 @@ import re
 
 import pytest
 
-from custom_components.nestquest.completion import (
+from custom_components.nestquest.completion import append_event, instance_state
-    append_event,
-    complete_instance,
-    instance_state,
-)
 from custom_components.nestquest.dao_children import ChildrenDao
 from custom_components.nestquest.dao_instances import (
     EVENT_COMPLETED,
@@ -337,7 +333,7 @@ def test_module_never_exposes_update_or_delete() -> None:
     )
     assert "append_event" in public
     assert "instance_state" in public
-    assert "complete_instance" in public
+    assert "complete_instance" not in public
     assert "uncomplete_instance" not in public
     source = inspect.getsource(completion)
     assert "UPDATE" not in source
@@ -448,239 +444,3 @@ def test_instance_state_three_events_returns_done_and_preserves_rows(
         assert len(after) == 3
 
     _with_db(tmp_path, "state-three.db")(_body)
-
-
-# ---------------------------------------------------------------------------
-# complete_instance: append completed, no-op if already done
-# ---------------------------------------------------------------------------
-
-
-def test_complete_instance_appends_and_returns_done(tmp_path) -> None:
-    async def _body(database, child, instance):
-        events = CompletionEventsDao(database)
-        assert await events.list_by_instance(instance.id) == []
-        state = await complete_instance(
-            database,
-            instance.id,
-            actor_source="user",
-            actor_user_id="user-1",
-        )
-        assert state == "done"
-        assert await instance_state(database, instance.id) == "done"
-        rows = await events.list_by_instance(instance.id)
-        assert len(rows) == 1
-        assert rows[0].event_type == EVENT_COMPLETED
-        assert rows[0].actor_source == "user"
-        assert rows[0].actor_user_id == "user-1"
-        assert rows[0].was_on_time is True
-
-    _with_db(tmp_path, "complete-open.db")(_body)
-
-
-def test_complete_instance_already_done_is_noop(tmp_path) -> None:
-    async def _body(database, child, instance):
-        first = await complete_instance(
-            database,
-            instance.id,
-            actor_source="user",
-            actor_user_id="user-1",
-        )
-        events = CompletionEventsDao(database)
-        before = await events.list_by_instance(instance.id)
-        assert first == "done"
-        assert len(before) == 1
-        second = await complete_instance(
-            database,
-            instance.id,
-            actor_source="user",
-            actor_user_id="user-2",
-        )
-        after = await events.list_by_instance(instance.id)
-        assert second == "done"
-        assert after == before
-        assert after[0].actor_user_id == "user-1"
-
-    _with_db(tmp_path, "complete-noop.db")(_body)
-
-
-def test_complete_instance_unknown_instance_id_raises(tmp_path) -> None:
-    async def _body(database, child, instance):
-        with pytest.raises(ValueError, match="instance_id"):
-            await complete_instance(
-                database,
-                instance.id + 999,
-                actor_source="user",
-                actor_user_id="user-1",
-            )
-
-    _with_db(tmp_path, "complete-unknown.db")(_body)
-
-
-def test_complete_instance_rejects_non_integer_id() -> None:
-    async def _body():
-        with pytest.raises(ValueError, match="instance_id"):
-            await complete_instance(
-                object(),
-                "1",
-                actor_source="user",
-                actor_user_id="user-1",
-            )
-        with pytest.raises(ValueError, match="instance_id"):
-            await complete_instance(
-                object(),
-                True,
-                actor_source="user",
-                actor_user_id="user-1",
-            )
-
-    _run(_body())
-
-
-def test_complete_instance_after_uncompleted_appends(tmp_path) -> None:
-    async def _body(database, child, instance):
-        await append_event(
-            database,
-            instance.id,
-            child.id,
-            EVENT_COMPLETED,
-            actor_source="user",
-            actor_user_id="user-1",
-            was_on_time=True,
-        )
-        await append_event(
-            database,
-            instance.id,
-            child.id,
-            EVENT_UNCOMPLETED,
-            actor_source="user",
-            actor_user_id="user-1",
-        )
-        events = CompletionEventsDao(database)
-        before = await events.list_by_instance(instance.id)
-        assert await instance_state(database, instance.id) == "open"
-        state = await complete_instance(
-            database,
-            instance.id,
-            actor_source="panel",
-            actor_child_id=child.id,
-        )
-        assert state == "done"
-        after = await events.list_by_instance(instance.id)
-        assert len(after) == len(before) + 1
-        assert after[-1].event_type == EVENT_COMPLETED
-        assert after[-1].actor_source == "panel"
-        assert after[-1].was_on_time is True
-
-    _with_db(tmp_path, "complete-reopen.db")(_body)
-
-
-def test_complete_instance_now_pins_occurred_at(tmp_path) -> None:
-    async def _body(database, child, instance):
-        pinned = datetime.datetime(
-            2026, 9, 18, 8, 0, 0, tzinfo=datetime.timezone.utc
-        )
-        await complete_instance(
-            database,
-            instance.id,
-            actor_source="user",
-            actor_user_id="user-1",
-            now=pinned,
-        )
-        rows = await CompletionEventsDao(database).list_by_instance(
-            instance.id
-        )
-        assert rows[0].occurred_at == "2026-09-18T08:00:00+00:00"
-
-    _with_db(tmp_path, "complete-pinned.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 INSERT of whichever ``complete_instance`` starts first
-    — proving that caller holds the connection lock 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_complete_instance_concurrent_second_call_is_noop(tmp_path) -> None:
-    """Two racing completes cannot both append.
-
-    Deterministic gate: task A is paused at its INSERT (it holds the
-    connection lock); task B is then started and must queue.  A
-    finishes, B sees done and appends nothing.
-    """
-    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 / "complete-race.db")
-        try:
-            await apply_migrations(database)
-            children = ChildrenDao(database)
-            rules = ScheduleRulesDao(database)
-            definitions = QuestDefinitionsDao(database)
-            instances = QuestInstancesDao(database)
-            child = await children.create("Ada", _now_stamp())
-            rule = await rules.create("daily", D1)
-            definition = await definitions.create(
-                "Brush teeth",
-                rule.id,
-                _now_stamp(),
-                assignee_child_ids=[child.id],
-            )
-            instance = await instances.upsert(
-                definition.id,
-                child.id,
-                D1,
-                _now_stamp(),
-                window="morning",
-            )
-
-            armed["active"] = True
-            task_a = asyncio.ensure_future(
-                complete_instance(
-                    database,
-                    instance.id,
-                    actor_source="user",
-                    actor_user_id="user-1",
-                )
-            )
-            await a_started.wait()
-            armed["active"] = False
-            task_b = asyncio.ensure_future(
-                complete_instance(
-                    database,
-                    instance.id,
-                    actor_source="user",
-                    actor_user_id="user-2",
-                )
-            )
-            await asyncio.sleep(0)
-            gate_open.set()
-            state_a, state_b = await asyncio.gather(task_a, task_b)
-            assert state_a == "done"
-            assert state_b == "done"
-            rows = await CompletionEventsDao(database).list_by_instance(
-                instance.id
-            )
-            assert len(rows) == 1
-            assert rows[0].actor_user_id == "user-1"
-        finally:
-            await database.close()
-
-    _run(_main())