From ea6cf73bdce05a4a67797a1ed4036970f94352a1 Mon Sep 17 00:00:00 2001 From: Joshua King Date: Mon, 14 Sep 2026 08:10:41 -0400 Subject: [PATCH 1/4] feat: add typed DAO layer for task_instances and append-only completion_events (#21) --- custom_components/nestquest/dao_instances.py | 431 +++++++++++ tests/test_dao_children.py | 2 + tests/test_dao_instances.py | 706 +++++++++++++++++++ tests/test_dao_rules.py | 8 +- 4 files changed, 1143 insertions(+), 4 deletions(-) create mode 100644 custom_components/nestquest/dao_instances.py create mode 100644 tests/test_dao_instances.py diff --git a/custom_components/nestquest/dao_instances.py b/custom_components/nestquest/dao_instances.py new file mode 100644 index 0000000..1f52c0e --- /dev/null +++ b/custom_components/nestquest/dao_instances.py @@ -0,0 +1,431 @@ +"""Typed DAO layer for task_instances and the append-only completion_events. + +All SQL for these tables lives in this module per the feature +guardrails: callers get typed dataclasses back and never see raw rows +or SQL. Every method is async and runs through +:class:`~.db.NestQuestDatabase`, so each statement executes on the HA +executor and the event loop never blocks. + +Append-only policy (Feature 08, D-005): ``completion_events`` is +strictly append-only. This module is where that property is enforced +mechanically — the completion-events DAO below exposes NO update or +delete method at all; completions append, un-completions append a +separate reversal event, and instance state derives from the latest +event. A package-wide guard test (bottom of the test module) keeps +mutation SQL for this table out of every other module. + +Upsert semantics: ``TaskInstancesDao.upsert`` is idempotent on +(definition_id, due_date) — re-running materialization for a date +whose instance already exists updates nothing that matters +(child_id/due_time are refreshed only in the degenerate re-generation +case where they changed) and never duplicates a row. The schema's +UNIQUE(definition_id, due_date) is the last line of defense. + +``delete_future_uncompleted`` implements the Feature 06/07 rule that +reassignment and schedule edits regenerate future instances: it +removes only instances at or after the cutoff that have NO completion +event, inside one transaction, so an instance completed mid-call can +never be deleted. Instances in the past are never touched (Feature 07 +guardrail: never delete history), and no method here deletes a single +instance that has a completion event. +""" +from __future__ import annotations + +from dataclasses import dataclass + +from .dao_children import _connection_lock +from .dao_rules import _validate_date +from .db import NestQuestDatabase + + +@dataclass(frozen=True) +class TaskInstanceRecord: + """One row of ``task_instances``.""" + + id: int + definition_id: int + child_id: int + due_date: str + due_time: str | None + generated_at: str + + +@dataclass(frozen=True) +class CompletionEventRecord: + """One row of ``completion_events`` (append-only table).""" + + id: int + instance_id: int + child_id: int + event_type: str + actor_source: str + actor_user_id: str | None + occurred_at: str + was_on_time: bool | None + + +_INSTANCE_COLUMNS = ( + "id, definition_id, child_id, due_date, due_time, generated_at" +) +_EVENT_COLUMNS = ( + "id, instance_id, child_id, event_type, actor_source, actor_user_id, " + "occurred_at, was_on_time" +) + +#: The only event types the append path accepts; the schema CHECK is +#: the backstop, this is the DAO's typed front door. +EVENT_COMPLETED = "completed" +EVENT_UNCOMPLETED = "uncompleted" + + +def _instance_from_row(row: tuple) -> TaskInstanceRecord: + return TaskInstanceRecord( + id=row[0], + definition_id=row[1], + child_id=row[2], + due_date=row[3], + due_time=row[4], + generated_at=row[5], + ) + + +def _event_from_row(row: tuple) -> CompletionEventRecord: + return CompletionEventRecord( + id=row[0], + instance_id=row[1], + child_id=row[2], + event_type=row[3], + actor_source=row[4], + actor_user_id=row[5], + occurred_at=row[6], + was_on_time=None if row[7] is None else bool(row[7]), + ) + + +class TaskInstancesDao: + """Typed async access to the ``task_instances`` table.""" + + def __init__(self, database: NestQuestDatabase) -> None: + self._database = database + + async def upsert( + self, + definition_id: int, + child_id: int, + due_date: str, + generated_at: str, + *, + due_time: str | None = None, + ) -> TaskInstanceRecord: + """Create the dated instance for (definition_id, due_date) idempotently. + + Running materialization twice for the same definition/date + yields ONE row: the ON CONFLICT path refreshes the + generation-time snapshot columns (child_id, due_time) and + regenerates the stamp, but can never create a duplicate — the + schema UNIQUE(definition_id, due_date) backs this up. The + definition must exist (validated atomically under the + connection lock so a concurrent definition deactivation or + child deletion cannot slip between check and insert). + """ + _validate_date(due_date, "due_date") + async with _connection_lock(self._database): + async with self._database.transaction(): + definition = await self._database.fetch_one( + "SELECT child_id FROM task_definitions WHERE id = ?", + (definition_id,), + ) + if definition is None: + raise ValueError( + f"task definition {definition_id} does not exist" + ) + if definition[0] != child_id: + raise ValueError( + f"task definition {definition_id} is assigned to " + f"child {definition[0]}, not {child_id}; instance " + "must carry the definition's assignee" + ) + child = await self._database.fetch_one( + "SELECT 1 FROM children WHERE id = ?", (child_id,) + ) + if child is None: + raise ValueError(f"child {child_id} does not exist") + await self._database.execute( + "INSERT INTO task_instances (definition_id, child_id, " + "due_date, due_time, generated_at) " + "VALUES (?, ?, ?, ?, ?) " + "ON CONFLICT (definition_id, due_date) DO UPDATE SET " + "child_id = excluded.child_id, " + "due_time = excluded.due_time, " + "generated_at = excluded.generated_at", + ( + definition_id, + child_id, + due_date, + due_time, + generated_at, + ), + ) + instance = await self.get(definition_id, due_date) + assert instance is not None + return instance + + async def get( + self, definition_id: int, due_date: str + ) -> TaskInstanceRecord | None: + """Return the instance for (definition_id, due_date), or None.""" + row = await self._database.fetch_one( + f"SELECT {_INSTANCE_COLUMNS} FROM task_instances " + "WHERE definition_id = ? AND due_date = ?", + (definition_id, due_date), + ) + return _instance_from_row(row) if row is not None else None + + async def get_by_id(self, instance_id: int) -> TaskInstanceRecord | None: + """Return the instance with ``instance_id``, or None.""" + row = await self._database.fetch_one( + f"SELECT {_INSTANCE_COLUMNS} FROM task_instances WHERE id = ?", + (instance_id,), + ) + return _instance_from_row(row) if row is not None else None + + async def list_by_child_and_date( + self, child_id: int, due_date: str + ) -> list[TaskInstanceRecord]: + """Return the child's instances due exactly on ``due_date``.""" + _validate_date(due_date, "due_date") + rows = await self._database.fetch_all( + f"SELECT {_INSTANCE_COLUMNS} FROM task_instances " + "WHERE child_id = ? AND due_date = ? ORDER BY id", + (child_id, due_date), + ) + return [_instance_from_row(row) for row in rows] + + async def list_by_date_range( + self, + child_id: int, + range_start: str, + range_end: str, + ) -> list[TaskInstanceRecord]: + """Return the child's instances with due_date in the closed range.""" + _validate_date(range_start, "range_start") + _validate_date(range_end, "range_end") + if range_end < range_start: + raise ValueError( + "range_end must be on or after range_start, got " + f"{range_end!r} < {range_start!r}" + ) + rows = await self._database.fetch_all( + f"SELECT {_INSTANCE_COLUMNS} FROM task_instances " + "WHERE child_id = ? AND due_date >= ? AND due_date <= ? " + "ORDER BY due_date, id", + (child_id, range_start, range_end), + ) + return [_instance_from_row(row) for row in rows] + + async def delete_future_uncompleted( + self, definition_id: int, cutoff_date: str + ) -> int: + """Delete the definition's instances at/after cutoff with no events. + + Implements regeneration after a definition edit or reassignment + (Features 06/07): only future, never-completed instances are + removed, inside one transaction — an instance that gains a + completion event mid-flight cannot be deleted. Instances + before the cutoff and any instance with a completion event are + never touched. Returns the number deleted. + """ + _validate_date(cutoff_date, "cutoff_date") + async with _connection_lock(self._database): + async with self._database.transaction(): + result = await self._database.execute( + "DELETE FROM task_instances WHERE definition_id = ? " + "AND due_date >= ? AND NOT EXISTS (" + " SELECT 1 FROM completion_events " + " WHERE instance_id = task_instances.id" + ")", + (definition_id, cutoff_date), + ) + return result.rowcount + + +class CompletionEventsDao: + """Typed append-only access to the ``completion_events`` table. + + APPEND-ONLY BY CONSTRUCTION: this class exposes ``append`` and the + three read methods below, and NOTHING else. There is no update, + no delete, no upsert — completions and un-completions both arrive + through :meth:`append` as new rows, and history is immutable + (Feature 08 done-condition: no code path updates or deletes an + event). The package-wide leak-guard test additionally fails if + mutation SQL for this table ever appears in any module. + """ + + def __init__(self, database: NestQuestDatabase) -> None: + self._database = database + + async def append( + self, + instance_id: int, + child_id: int, + event_type: str, + actor_source: str, + occurred_at: str, + was_on_time: bool, + *, + actor_user_id: str | None = None, + ) -> CompletionEventRecord: + """Append one completion or un-completion event. + + ``actor_source`` is 'user' (with ``actor_user_id`` set) or + 'panel' (no user id — the tapped child is ``child_id``); the + actor-pair CHECK enforces this shape at the schema level. + ``was_on_time`` is required for completions; for un-completions + it may be None or the reversed completion's flag (Feature 08 + still deciding), so pass ``was_on_time=...`` or omit via + ``None``. The instance and child must exist — validated + atomically under the connection lock. + """ + if event_type not in (EVENT_COMPLETED, EVENT_UNCOMPLETED): + raise ValueError( + f"event_type must be '{EVENT_COMPLETED}' or " + f"'{EVENT_UNCOMPLETED}', got {event_type!r}" + ) + if actor_source not in ("user", "panel"): + raise ValueError( + f"actor_source must be 'user' or 'panel', got " + f"{actor_source!r}" + ) + if actor_source == "user" and not actor_user_id: + raise ValueError( + "actor_source 'user' requires actor_user_id" + ) + if actor_source == "panel" and actor_user_id is not None: + raise ValueError( + "actor_source 'panel' must not carry actor_user_id" + ) + if event_type == EVENT_COMPLETED and was_on_time is None: + raise ValueError( + "a completed event requires was_on_time (True/False)" + ) + async with _connection_lock(self._database): + async with self._database.transaction(): + instance = await self._database.fetch_one( + "SELECT child_id FROM task_instances WHERE id = ?", + (instance_id,), + ) + if instance is None: + raise ValueError( + f"task instance {instance_id} does not exist" + ) + if instance[0] != child_id: + raise ValueError( + f"task instance {instance_id} belongs to child " + f"{instance[0]}, not {child_id}" + ) + child = await self._database.fetch_one( + "SELECT 1 FROM children WHERE id = ?", (child_id,) + ) + if child is None: + raise ValueError(f"child {child_id} does not exist") + result = await self._database.execute( + "INSERT INTO completion_events (instance_id, child_id, " + "event_type, actor_source, actor_user_id, occurred_at, " + "was_on_time) VALUES (?, ?, ?, ?, ?, ?, ?)", + ( + instance_id, + child_id, + event_type, + actor_source, + actor_user_id, + occurred_at, + None if was_on_time is None else int(was_on_time), + ), + ) + event = await self._database.fetch_one( + f"SELECT {_EVENT_COLUMNS} FROM completion_events " + "WHERE id = ?", + (result.lastrowid,), + ) + assert event is not None + return _event_from_row(event) + + async def list_by_instance( + self, instance_id: int + ) -> list[CompletionEventRecord]: + """Return every event for the instance, in append order.""" + rows = await self._database.fetch_all( + f"SELECT {_EVENT_COLUMNS} FROM completion_events " + "WHERE instance_id = ? ORDER BY id", + (instance_id,), + ) + return [_event_from_row(row) for row in rows] + + async def list_by_child_and_date_range( + self, + child_id: int, + range_start: str, + range_end: str, + *, + due_date_scope: bool = True, + ) -> list[CompletionEventRecord]: + """Return the child's events for instances due in the closed range. + + Scopes by the INSTANCE's due date (the day the task was owed), + not the event's wall-clock timestamp: history reporting asks + "what happened to the tasks due that day" (Feature 13). Set + ``due_date_scope=False`` to filter by ``occurred_at`` date + instead. Ordered by instance, then event id. + """ + _validate_date(range_start, "range_start") + _validate_date(range_end, "range_end") + if range_end < range_start: + raise ValueError( + "range_end must be on or after range_start, got " + f"{range_end!r} < {range_start!r}" + ) + if due_date_scope: + predicate = ( + "SELECT id FROM task_instances WHERE child_id = ? " + "AND due_date >= ? AND due_date <= ?" + ) + parameters: list[object] = [child_id, range_start, range_end] + else: + predicate = ( + "SELECT id FROM task_instances WHERE child_id = ?" + ) + parameters = [child_id] + occurred_start = f"{range_start}T00:00:00" + occurred_end = f"{range_end}T23:59:59.999999" + query = ( + f"SELECT {_EVENT_COLUMNS} FROM completion_events " + "WHERE child_id = ? AND occurred_at >= ? " + "AND occurred_at <= ? ORDER BY instance_id, id" + ) + rows = await self._database.fetch_all( + query, [child_id, occurred_start, occurred_end] + ) + return [_event_from_row(row) for row in rows] + rows = await self._database.fetch_all( + f"SELECT {_EVENT_COLUMNS} FROM completion_events " + f"WHERE instance_id IN ({predicate}) " + "ORDER BY instance_id, id", + tuple(parameters), + ) + return [_event_from_row(row) for row in rows] + + async def get_latest_for_instance( + self, instance_id: int + ) -> CompletionEventRecord | None: + """Return the instance's most recent event, or None if untouched. + + Instance current state derives from this event (Feature 08): + 'completed' means done, 'uncompleted' as the latest means open + again, None means never completed. + """ + row = await self._database.fetch_one( + f"SELECT {_EVENT_COLUMNS} FROM completion_events " + "WHERE instance_id = ? ORDER BY id DESC LIMIT 1", + (instance_id,), + ) + return _event_from_row(row) if row is not None else None \ No newline at end of file diff --git a/tests/test_dao_children.py b/tests/test_dao_children.py index 89d4911..a25514a 100644 --- a/tests/test_dao_children.py +++ b/tests/test_dao_children.py @@ -484,6 +484,8 @@ def test_children_and_admin_sql_lives_only_in_dao_module() -> None: "custom_components/nestquest/dao_presence.py", # validates the # schedule/override child exists before writing (same FK shape # as dao_rules: one existence SELECT per write) + "custom_components/nestquest/dao_instances.py", # validates the + # instance/event child exists before writing (same FK shape) "custom_components/nestquest/schema.py", # declares the DDL "custom_components/nestquest/migrations.py", # applies the DDL "tests/test_schema.py", # tests the DDL diff --git a/tests/test_dao_instances.py b/tests/test_dao_instances.py new file mode 100644 index 0000000..7644a77 --- /dev/null +++ b/tests/test_dao_instances.py @@ -0,0 +1,706 @@ +"""Tests for dao_instances.py: task_instances + append-only completion_events.""" +from __future__ import annotations + +import asyncio +import sqlite3 + +import pytest + +from custom_components.nestquest.dao_children import ChildrenDao +from custom_components.nestquest.dao_instances import ( + CompletionEventRecord, + CompletionEventsDao, + TaskInstanceRecord, + TaskInstancesDao, +) +from custom_components.nestquest.dao_rules import ScheduleRulesDao, TaskDefinitionsDao +from custom_components.nestquest.db import NestQuestDatabase +from custom_components.nestquest.migrations import apply_migrations + + +def _run(coro): + return asyncio.new_event_loop().run_until_complete(coro) + + +def _make_hass_mock(): + from unittest.mock import AsyncMock, MagicMock + + hass = MagicMock() + hass.async_add_executor_job = AsyncMock(side_effect=(lambda fn, *a: fn(*a))) + return hass + + +NOW = "2026-09-14T12:00:00+00:00" + + +async def _prepare(path) -> tuple: + database = NestQuestDatabase(_make_hass_mock()) + await database.open(path) + await apply_migrations(database) + children = ChildrenDao(database) + rules = ScheduleRulesDao(database) + definitions = TaskDefinitionsDao(database) + instances = TaskInstancesDao(database) + events = CompletionEventsDao(database) + child = await children.create("Ada", NOW) + rule = await rules.create("daily", "2026-09-01") + definition = await definitions.create("Brush teeth", child.id, rule.id, NOW) + return ( + database, + children, + rules, + definitions, + instances, + events, + child, + definition, + ) + + +def _with_db(tmp_path, name): + def _run_test(body): + async def _main(): + prepared = await _prepare(tmp_path / name) + try: + return await body(*prepared) + finally: + await prepared[0].close() + + return _run(_main()) + + return _run_test + + +# --------------------------------------------------------------------------- +# task_instances: upsert idempotency +# --------------------------------------------------------------------------- + + +def test_upsert_creates_then_is_idempotent(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + first = await instances.upsert( + definition.id, child.id, "2026-09-15", NOW + ) + assert isinstance(first, TaskInstanceRecord) + second = await instances.upsert( + definition.id, child.id, "2026-09-15", NOW + ) + assert second.id == first.id + rows = await database.fetch_one( + "SELECT COUNT(*) FROM task_instances" + ) + assert rows == (1,) + return second + + _with_db(tmp_path, "upsert-idempotent.db")(_body) + + +def test_upsert_different_dates_create_distinct_rows(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + a = await instances.upsert(definition.id, child.id, "2026-09-15", NOW) + b = await instances.upsert(definition.id, child.id, "2026-09-16", NOW) + assert a.id != b.id + return (a, b) + + _with_db(tmp_path, "upsert-multi-date.db")(_body) + + +def test_upsert_refreshes_snapshot_columns_on_conflict(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + first = await instances.upsert( + definition.id, child.id, "2026-09-15", NOW, due_time="08:00" + ) + second = await instances.upsert( + definition.id, + child.id, + "2026-09-15", + "2026-09-14T13:00:00+00:00", + due_time="08:30", + ) + assert second.id == first.id + assert second.due_time == "08:30" + assert second.generated_at == "2026-09-14T13:00:00+00:00" + return second + + _with_db(tmp_path, "upsert-refresh.db")(_body) + + +def test_upsert_rejects_wrong_child_for_definition(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + other = await children.create("Bo", NOW) + with pytest.raises(ValueError, match="assigned to"): + await instances.upsert(definition.id, other.id, "2026-09-15", NOW) + return None + + _with_db(tmp_path, "upsert-wrong-child.db")(_body) + + +def test_upsert_rejects_unknown_definition_and_child(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + with pytest.raises(ValueError, match="definition 999"): + await instances.upsert(999, child.id, "2026-09-15", NOW) + # Unknown child: the definition-assignee check fires first and + # is the one that matters (an instance must carry its + # definition's assignee). To reach the child-existence check + # at all, the definition would have to claim child 999 — an + # FK-impossible state — so the child check only guards direct + # callers with a matching fake assignee; assert the guard + # order instead of forcing the FK-impossible state. + rule = await rules.create("daily", "2026-09-01") + other_definition = await definitions.create( + "X", child.id, rule.id, NOW + ) + with pytest.raises(ValueError, match="not 999"): + await instances.upsert(other_definition.id, 999, "2026-09-15", NOW) + return None + + _with_db(tmp_path, "upsert-unknown.db")(_body) + + +def test_upsert_malformed_due_date_raises(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + for bad in ("not-a-date", "2026-9-15", ""): + with pytest.raises(ValueError, match="YYYY-MM-DD"): + await instances.upsert(definition.id, child.id, bad, NOW) + return None + + _with_db(tmp_path, "upsert-bad-date.db")(_body) + + +# --------------------------------------------------------------------------- +# task_instances: get / list +# --------------------------------------------------------------------------- + + +def test_get_by_definition_and_date_round_trip(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + created = await instances.upsert( + definition.id, child.id, "2026-09-15", NOW, due_time="17:30" + ) + fetched = await instances.get(definition.id, "2026-09-15") + assert fetched == created + assert fetched.due_time == "17:30" + assert await instances.get(definition.id, "2026-09-16") is None + return fetched + + _with_db(tmp_path, "get-round-trip.db")(_body) + + +def test_get_by_id_round_trip(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + created = await instances.upsert(definition.id, child.id, "2026-09-15", NOW) + fetched = await instances.get_by_id(created.id) + assert fetched == created + assert await instances.get_by_id(999) is None + return fetched + + _with_db(tmp_path, "get-by-id.db")(_body) + + +def test_list_by_child_and_date_filters(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + other = await children.create("Bo", NOW) + rule = await rules.create("daily", "2026-09-01") + other_definition = await definitions.create( + "Make bed", other.id, rule.id, NOW + ) + await instances.upsert(definition.id, child.id, "2026-09-15", NOW) + await instances.upsert(definition.id, child.id, "2026-09-16", NOW) + await instances.upsert( + other_definition.id, other.id, "2026-09-15", NOW + ) + mine = await instances.list_by_child_and_date(child.id, "2026-09-15") + assert len(mine) == 1 + assert mine[0].child_id == child.id + assert mine[0].due_date == "2026-09-15" + return mine + + _with_db(tmp_path, "list-by-child-date.db")(_body) + + +def test_list_by_date_range_filters_and_orders(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + for day in ("2026-09-10", "2026-09-12", "2026-09-14", "2026-09-20"): + await instances.upsert(definition.id, child.id, day, NOW) + found = await instances.list_by_date_range( + child.id, "2026-09-11", "2026-09-15" + ) + assert [i.due_date for i in found] == ["2026-09-12", "2026-09-14"] + edge = await instances.list_by_date_range( + child.id, "2026-09-12", "2026-09-12" + ) + assert [i.due_date for i in edge] == ["2026-09-12"] + return found + + _with_db(tmp_path, "list-by-range.db")(_body) + + +def test_list_by_date_range_rejects_inverted_range(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + with pytest.raises(ValueError, match="on or after"): + await instances.list_by_date_range( + child.id, "2026-09-15", "2026-09-10" + ) + return None + + _with_db(tmp_path, "list-inverted.db")(_body) + + +# --------------------------------------------------------------------------- +# task_instances: delete_future_uncompleted +# --------------------------------------------------------------------------- + + +def test_delete_future_uncompleted_removes_only_eligible(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + await instances.upsert(definition.id, child.id, "2026-09-10", NOW) + future_open = await instances.upsert( + definition.id, child.id, "2026-09-20", NOW + ) + await instances.upsert(definition.id, child.id, "2026-09-21", NOW) + deleted = await instances.delete_future_uncompleted( + definition.id, "2026-09-20" + ) + assert deleted == 2 + assert await instances.get(definition.id, "2026-09-10") is not None + assert await instances.get(definition.id, "2026-09-20") is None + assert await instances.get(definition.id, "2026-09-21") is None + return deleted + + _with_db(tmp_path, "delete-future.db")(_body) + + +def test_delete_future_uncompleted_spares_completed(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + completed = await instances.upsert( + definition.id, child.id, "2026-09-20", NOW + ) + open_one = await instances.upsert( + definition.id, child.id, "2026-09-21", NOW + ) + await events.append( + completed.id, child.id, "completed", "user", NOW, True, + actor_user_id="user-1", + ) + deleted = await instances.delete_future_uncompleted( + definition.id, "2026-09-20" + ) + assert deleted == 1 + assert await instances.get_by_id(completed.id) is not None, ( + "an instance with a completion event must never be deleted" + ) + assert await instances.get_by_id(open_one.id) is None + return deleted + + _with_db(tmp_path, "delete-spares-completed.db")(_body) + + +def test_delete_future_uncompleted_spares_past(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + await instances.upsert(definition.id, child.id, "2026-09-10", NOW) + await instances.upsert(definition.id, child.id, "2026-09-11", NOW) + deleted = await instances.delete_future_uncompleted( + definition.id, "2026-09-11" + ) + assert deleted == 1 + assert await instances.get(definition.id, "2026-09-10") is not None + return deleted + + _with_db(tmp_path, "delete-spares-past.db")(_body) + + +def test_delete_future_uncompleted_other_definitions_untouched( + tmp_path, +) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + other = await definitions.create( + "Make bed", child.id, (await rules.create("daily", "2026-09-01")).id, + NOW, + ) + await instances.upsert(definition.id, child.id, "2026-09-20", NOW) + await instances.upsert(other.id, child.id, "2026-09-20", NOW) + deleted = await instances.delete_future_uncompleted( + definition.id, "2026-09-20" + ) + assert deleted == 1 + assert await instances.get(other.id, "2026-09-20") is not None + return deleted + + _with_db(tmp_path, "delete-scoped.db")(_body) + + +# --------------------------------------------------------------------------- +# completion_events: append +# --------------------------------------------------------------------------- + + +def test_append_completion_round_trips(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + instance = await instances.upsert( + definition.id, child.id, "2026-09-15", NOW + ) + event = await events.append( + instance.id, child.id, "completed", "user", NOW, True, + actor_user_id="user-1", + ) + assert isinstance(event, CompletionEventRecord) + assert event.event_type == "completed" + assert event.actor_source == "user" + assert event.actor_user_id == "user-1" + assert event.was_on_time is True + return event + + _with_db(tmp_path, "append-completion.db")(_body) + + +def test_append_panel_event_round_trips(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + instance = await instances.upsert( + definition.id, child.id, "2026-09-15", NOW + ) + event = await events.append( + instance.id, child.id, "completed", "panel", NOW, False, + actor_user_id=None, + ) + assert event.actor_source == "panel" + assert event.actor_user_id is None + assert event.was_on_time is False + return event + + _with_db(tmp_path, "append-panel.db")(_body) + + +def test_append_uncompleted_with_optional_on_time(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + instance = await instances.upsert( + definition.id, child.id, "2026-09-15", NOW + ) + reversal = await events.append( + instance.id, child.id, "uncompleted", "user", NOW, None, + actor_user_id="user-1", + ) + assert reversal.event_type == "uncompleted" + assert reversal.was_on_time is None + carried = await events.append( + instance.id, child.id, "uncompleted", "user", NOW, True, + actor_user_id="user-1", + ) + assert carried.was_on_time is True + return (reversal, carried) + + _with_db(tmp_path, "append-uncompleted.db")(_body) + + +def test_append_validation_rejects_bad_shapes(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + instance = await instances.upsert( + definition.id, child.id, "2026-09-15", NOW + ) + with pytest.raises(ValueError, match="event_type"): + await events.append( + instance.id, child.id, "missed", "user", NOW, True, + actor_user_id="user-1", + ) + with pytest.raises(ValueError, match="actor_source"): + await events.append( + instance.id, child.id, "completed", "kiosk", NOW, True, + actor_user_id="user-1", + ) + with pytest.raises(ValueError, match="requires actor_user_id"): + await events.append( + instance.id, child.id, "completed", "user", NOW, True, + actor_user_id=None, + ) + with pytest.raises(ValueError, match="must not carry"): + await events.append( + instance.id, child.id, "completed", "panel", NOW, True, + actor_user_id="user-1", + ) + with pytest.raises(ValueError, match="requires was_on_time"): + await events.append( + instance.id, child.id, "completed", "panel", NOW, None + ) + return None + + _with_db(tmp_path, "append-validation.db")(_body) + + +def test_append_rejects_unknown_instance_or_child(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + with pytest.raises(ValueError, match="instance 999"): + await events.append( + 999, child.id, "completed", "user", NOW, True, + actor_user_id="user-1", + ) + instance = await instances.upsert( + definition.id, child.id, "2026-09-15", NOW + ) + with pytest.raises(ValueError, match="belongs to child"): + await events.append( + instance.id, 999, "completed", "user", NOW, True, + actor_user_id="user-1", + ) + return None + + _with_db(tmp_path, "append-unknown.db")(_body) + + +# --------------------------------------------------------------------------- +# completion_events: reads and append-only semantics +# --------------------------------------------------------------------------- + + +def test_completed_uncompleted_recompleted_three_ordered_events( + tmp_path, +) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + instance = await instances.upsert( + definition.id, child.id, "2026-09-15", NOW + ) + await events.append( + instance.id, child.id, "completed", "panel", NOW, True + ) + await events.append( + instance.id, child.id, "uncompleted", "user", NOW, True, + actor_user_id="user-1", + ) + await events.append( + instance.id, child.id, "completed", "panel", NOW, False + ) + history = await events.list_by_instance(instance.id) + assert [e.event_type for e in history] == [ + "completed", + "uncompleted", + "completed", + ] + assert [e.id for e in history] == sorted(e.id for e in history) + latest = await events.get_latest_for_instance(instance.id) + assert latest.event_type == "completed" + assert latest.was_on_time is False + return history + + _with_db(tmp_path, "three-events.db")(_body) + + +def test_get_latest_for_instance_none_when_untouched(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + instance = await instances.upsert( + definition.id, child.id, "2026-09-15", NOW + ) + return await events.get_latest_for_instance(instance.id) + + assert _with_db(tmp_path, "latest-none.db")(_body) is None + + +def test_get_latest_returns_uncompleted_when_reversed(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + instance = await instances.upsert( + definition.id, child.id, "2026-09-15", NOW + ) + await events.append( + instance.id, child.id, "completed", "panel", NOW, True + ) + await events.append( + instance.id, child.id, "uncompleted", "user", NOW, True, + actor_user_id="user-1", + ) + latest = await events.get_latest_for_instance(instance.id) + assert latest.event_type == "uncompleted" + return latest + + _with_db(tmp_path, "latest-reversed.db")(_body) + + +def test_list_by_child_and_date_range_scopes_by_due_date(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + early = await instances.upsert( + definition.id, child.id, "2026-09-10", NOW + ) + inside = await instances.upsert( + definition.id, child.id, "2026-09-12", NOW + ) + late = await instances.upsert( + definition.id, child.id, "2026-09-20", NOW + ) + await events.append( + early.id, child.id, "completed", "panel", NOW, True + ) + await events.append( + inside.id, child.id, "completed", "panel", NOW, False + ) + await events.append( + inside.id, child.id, "uncompleted", "user", NOW, True, + actor_user_id="user-1", + ) + await events.append( + late.id, child.id, "completed", "panel", NOW, True + ) + found = await events.list_by_child_and_date_range( + child.id, "2026-09-11", "2026-09-15" + ) + assert [e.id for e in found] == [2, 3] # inside's two events + return found + + _with_db(tmp_path, "events-by-range.db")(_body) + + +def test_list_events_occurred_at_scope_variant(tmp_path) -> None: + async def _body(database, children, rules, definitions, instances, + events, child, definition): + late_due = await instances.upsert( + definition.id, child.id, "2026-09-20", NOW + ) + # Completed long after its due date: due-date scope misses it, + # occurred-at scope catches it. + await events.append( + late_due.id, child.id, "completed", "panel", NOW, False + ) + by_due = await events.list_by_child_and_date_range( + child.id, "2026-09-01", "2026-09-15" + ) + assert by_due == [] + by_occurrence = await events.list_by_child_and_date_range( + child.id, "2026-09-14", "2026-09-14", due_date_scope=False + ) + assert len(by_occurrence) == 1 + assert by_occurrence[0].instance_id == late_due.id + return by_occurrence + + _with_db(tmp_path, "events-occurred-scope.db")(_body) + + +def test_completion_events_dao_has_no_update_or_delete(tmp_path) -> None: + """Done-condition: NO update or delete method exists at all.""" + async def _body(database, children, rules, definitions, instances, + events, child, definition): + import inspect + + methods = { + name + for name, _ in inspect.getmembers( + CompletionEventsDao, inspect.isfunction + ) + if not name.startswith("__") + } + forbidden = { + name for name in methods + if name.startswith(("update", "delete", "set", "clear", + "remove", "modify", "edit", "upsert")) + } + assert forbidden == set(), ( + f"append-only DAO exposed mutation methods: {forbidden}" + ) + assert methods == { + "append", + "list_by_instance", + "list_by_child_and_date_range", + "get_latest_for_instance", + } + return methods + + _with_db(tmp_path, "events-no-mutation.db")(_body) + + +def test_no_mutation_sql_for_completion_events_anywhere() -> None: + """Package-wide guardrail: UPDATE/DELETE ... completion_events must + not appear in ANY module or test — the append-only property is + enforced here, not just in the DAO class docstring. + """ + import re + from pathlib import Path + + package = Path( + __import__( + "custom_components.nestquest", fromlist=["__file__"] + ).__file__ + ).parent + repo_root = package.parent.parent + mutation_pattern = re.compile( + r"(UPDATE|DELETE\s+FROM)\s+[`'\"]*(\[)?completion_events\b", + re.IGNORECASE, + ) + offenders: list[str] = [] + for root in (package, repo_root / "tests"): + for py in sorted(root.rglob("*.py")): + if mutation_pattern.search(py.read_text()): + offenders.append(py.relative_to(repo_root).as_posix()) + assert offenders == [], ( + f"completion_events mutation SQL found in: {offenders}" + ) + + +def test_append_concurrent_events_serialize(tmp_path) -> None: + """Concurrent appends for the same instance all land, in order.""" + async def _main(): + database = NestQuestDatabase(_make_hass_mock()) + await database.open(tmp_path / "append-race.db") + try: + await apply_migrations(database) + prepared = await _prepare_in(database) + _, children, _, _, instances, events, child, definition = ( + prepared + ) + instance = await instances.upsert( + definition.id, child.id, "2026-09-15", NOW + ) + tasks = [ + asyncio.ensure_future( + events.append( + instance.id, child.id, "completed", "user", + f"2026-09-15T08:0{index}:00+00:00", True, + actor_user_id=f"user-{index}", + ) + ) + for index in range(5) + ] + await asyncio.sleep(0) + appended = await asyncio.gather(*tasks) + history = await events.list_by_instance(instance.id) + assert [e.id for e in history] == [ + e.id for e in sorted(appended, key=lambda x: x.id) + ] + assert len(history) == 5 + return history + finally: + await database.close() + + async def _prepare_in(database): + children = ChildrenDao(database) + rules = ScheduleRulesDao(database) + definitions = TaskDefinitionsDao(database) + instances = TaskInstancesDao(database) + events = CompletionEventsDao(database) + child = await children.create("Ada", NOW) + rule = await rules.create("daily", "2026-09-01") + definition = await definitions.create( + "Brush teeth", child.id, rule.id, NOW + ) + return (database, children, rules, definitions, instances, + events, child, definition) + + history = _run(_main()) + assert history[-1].occurred_at == "2026-09-15T08:04:00+00:00" \ No newline at end of file diff --git a/tests/test_dao_rules.py b/tests/test_dao_rules.py index 50ec5bc..429498c 100644 --- a/tests/test_dao_rules.py +++ b/tests/test_dao_rules.py @@ -789,15 +789,15 @@ def test_rules_and_definitions_sql_lives_only_in_dao_module() -> None: allowed = { "custom_components/nestquest/dao_rules.py", # this DAO + "custom_components/nestquest/dao_instances.py", # validates the + # definition exists + fetches its assignee before generating an + # instance (the instance must snapshot the definition's child) "custom_components/nestquest/schema.py", # declares the DDL "custom_components/nestquest/migrations.py", # applies the DDL "tests/test_schema.py", # tests the DDL "tests/test_migrations.py", # tests migration application "tests/test_dao_rules.py", # this file, scanned separately - # dao_children.py is NOT exempt: it holds no SQL for these - # tables (its FK reference check names task_definitions only - # inside delete_if_unreferenced's comment-free SQL? No — that - # lives HERE in dao_rules). Any future leak there must fail. + "tests/test_dao_instances.py", # instances guard, own scope } sql_pattern = re.compile( r"(FROM|INTO|UPDATE|DELETE\s+FROM|JOIN)\s+[`'\"]*(\[)?" -- 2.55.0 From e48123e488d6620681ba2e6f4bea5e01b88295d5 Mon Sep 17 00:00:00 2001 From: Joshua King Date: Mon, 14 Sep 2026 08:17:48 -0400 Subject: [PATCH 2/4] fix: immutable completed instances, no past generation, strict UTC timestamps, hardened append-only guards - upsert refuses due_dates before today (never generate in the past) and refuses regeneration of any instance carrying a completion event (immutability); the ON CONFLICT refresh only ever touches open instances - delete_future_uncompleted clamps the cutoff to today so backdating cannot rewrite past open instances - occurred_at enforces ONE strict UTC shape (YYYY-MM-DDTHH:MM:SS+00:00) at append; occurred-at range scope uses exact half-open bounds - was_on_time: bool | None with type(was_on_time) is bool validation - 0.5 silently coercing to False is rejected - append-only guards hardened: package-wide mutation regex covers UPDATE OR IGNORE/qualified names/quotes, DAO INSERT exempted narrowly; behavioral call-interception test proves only INSERT/SELECT ever reach the DB for completion_events; compensating self-scan for the instances test file's rules-table exemption --- custom_components/nestquest/dao_instances.py | 137 +++++-- tests/test_dao_instances.py | 365 +++++++++++++++---- tests/test_dao_rules.py | 44 +++ 3 files changed, 444 insertions(+), 102 deletions(-) diff --git a/custom_components/nestquest/dao_instances.py b/custom_components/nestquest/dao_instances.py index 1f52c0e..f0b6e37 100644 --- a/custom_components/nestquest/dao_instances.py +++ b/custom_components/nestquest/dao_instances.py @@ -31,12 +31,55 @@ instance that has a completion event. """ from __future__ import annotations +import datetime from dataclasses import dataclass from .dao_children import _connection_lock from .dao_rules import _validate_date from .db import NestQuestDatabase +#: Timestamp policy for completion_events.occurred_at: strict UTC +#: ISO-8601 with explicit '+00:00' offset (module timestamp policy). +#: One accepted shape bounds lexicographic range queries correctly, so +#: the occurred-at scope does not guess at timestamp spellings. +_UTC_TIMESTAMP_FORMAT = "%Y-%m-%dT%H:%M:%S+00:00" + + +def _validate_utc_timestamp(value: str, field: str) -> None: + """Raise ValueError unless ``value`` is a strict UTC ISO-8601 stamp. + + Same strictness policy as _validate_date: parse with the exact + format, then round-trip the parsed value against the input so + non-padded or re-ordered shapes are rejected. The round-trip + strips the UTC offset (isoformat() omits it when the offset is + zero), so the suffix is appended before comparing. + """ + try: + parsed = datetime.datetime.strptime( + value, _UTC_TIMESTAMP_FORMAT + ) + except (TypeError, ValueError): + raise ValueError( + f"{field} must be a UTC ISO-8601 timestamp " + f"(YYYY-MM-DDTHH:MM:SS+00:00), got {value!r}" + ) from None + if parsed.isoformat() + "+00:00" != value: + raise ValueError( + f"{field} must be a strict UTC timestamp " + f"(YYYY-MM-DDTHH:MM:SS+00:00), got {value!r}" + ) + + +def _next_day_start(date_str: str) -> str: + """Return the UTC timestamp one second past ``date_str``'s last second. + + Gives range queries an exact half-open upper bound without + fractional-second guessing. + """ + day = datetime.date.fromisoformat(date_str) + next_day = day + datetime.timedelta(days=1) + return f"{next_day.isoformat()}T00:00:00+00:00" + @dataclass(frozen=True) class TaskInstanceRecord: @@ -123,12 +166,29 @@ class TaskInstancesDao: yields ONE row: the ON CONFLICT path refreshes the generation-time snapshot columns (child_id, due_time) and regenerates the stamp, but can never create a duplicate — the - schema UNIQUE(definition_id, due_date) backs this up. The - definition must exist (validated atomically under the + schema UNIQUE(definition_id, due_date) backs this up. + + Guardrails enforced here: + + - ``due_date`` must be TODAY or later: instances are never + generated in the past (Feature 07). The caller's + materialization horizon is future-dated by definition. + - An existing instance WITH a completion event is IMMUTABLE: + the conflict path must not rewrite its snapshot columns + (Feature 07/08). The upsert refuses with ValueError in that + case — silently no-oping would mask a materialization bug. + + The definition must exist (validated atomically under the connection lock so a concurrent definition deactivation or child deletion cannot slip between check and insert). """ _validate_date(due_date, "due_date") + today = datetime.date.today().isoformat() + if due_date < today: + raise ValueError( + f"instances are never generated in the past: due_date " + f"{due_date!r} is before today {today!r}" + ) async with _connection_lock(self._database): async with self._database.transaction(): definition = await self._database.fetch_one( @@ -150,6 +210,18 @@ class TaskInstancesDao: ) if child is None: raise ValueError(f"child {child_id} does not exist") + existing = await self._database.fetch_one( + "SELECT 1 FROM completion_events " + "WHERE instance_id = (SELECT id FROM task_instances " + "WHERE definition_id = ? AND due_date = ?)", + (definition_id, due_date), + ) + if existing is not None: + raise ValueError( + f"instance for (definition {definition_id}, " + f"due_date {due_date!r}) already has a completion " + "event and is immutable; it cannot be regenerated" + ) await self._database.execute( "INSERT INTO task_instances (definition_id, child_id, " "due_date, due_time, generated_at) " @@ -226,16 +298,23 @@ class TaskInstancesDao: async def delete_future_uncompleted( self, definition_id: int, cutoff_date: str ) -> int: - """Delete the definition's instances at/after cutoff with no events. + """Delete the definition's open instances at/after cutoff. Implements regeneration after a definition edit or reassignment - (Features 06/07): only future, never-completed instances are - removed, inside one transaction — an instance that gains a - completion event mid-flight cannot be deleted. Instances - before the cutoff and any instance with a completion event are - never touched. Returns the number deleted. + (Features 06/07): 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: "future" + is measured against TODAY (materialization regenerates the + rolling horizon from now), so a caller cannot use this to + rewrite past open instances either — the day-rollover missed + sweep (Feature 11) handles past-due instances instead. Any + instance with a completion event is never touched, regardless + of its date. Returns the number deleted. """ _validate_date(cutoff_date, "cutoff_date") + today = datetime.date.today().isoformat() + effective_cutoff = max(cutoff_date, today) async with _connection_lock(self._database): async with self._database.transaction(): result = await self._database.execute( @@ -244,7 +323,7 @@ class TaskInstancesDao: " SELECT 1 FROM completion_events " " WHERE instance_id = task_instances.id" ")", - (definition_id, cutoff_date), + (definition_id, effective_cutoff), ) return result.rowcount @@ -271,7 +350,7 @@ class CompletionEventsDao: event_type: str, actor_source: str, occurred_at: str, - was_on_time: bool, + was_on_time: bool | None, *, actor_user_id: str | None = None, ) -> CompletionEventRecord: @@ -280,11 +359,13 @@ class CompletionEventsDao: ``actor_source`` is 'user' (with ``actor_user_id`` set) or 'panel' (no user id — the tapped child is ``child_id``); the actor-pair CHECK enforces this shape at the schema level. - ``was_on_time`` is required for completions; for un-completions - it may be None or the reversed completion's flag (Feature 08 - still deciding), so pass ``was_on_time=...`` or omit via - ``None``. The instance and child must exist — validated - atomically under the connection lock. + ``occurred_at`` must be the module's one strict UTC shape + (YYYY-MM-DDTHH:MM:SS+00:00) so lexicographic range queries + stay exact. ``was_on_time`` is required (True/False) for + completions; for un-completions it may be None or the reversed + completion's flag (Feature 08 still deciding). The instance + and child must exist — validated atomically under the + connection lock. """ if event_type not in (EVENT_COMPLETED, EVENT_UNCOMPLETED): raise ValueError( @@ -304,10 +385,16 @@ class CompletionEventsDao: raise ValueError( "actor_source 'panel' must not carry actor_user_id" ) + if was_on_time is not None and type(was_on_time) is not bool: + raise ValueError( + f"was_on_time must be True, False or None, got " + f"{was_on_time!r}" + ) if event_type == EVENT_COMPLETED and was_on_time is None: raise ValueError( "a completed event requires was_on_time (True/False)" ) + _validate_utc_timestamp(occurred_at, "occurred_at") async with _connection_lock(self._database): async with self._database.transaction(): instance = await self._database.fetch_one( @@ -391,19 +478,23 @@ class CompletionEventsDao: ) parameters: list[object] = [child_id, range_start, range_end] else: - predicate = ( - "SELECT id FROM task_instances WHERE child_id = ?" - ) - parameters = [child_id] - occurred_start = f"{range_start}T00:00:00" - occurred_end = f"{range_end}T23:59:59.999999" + # Occurred-at scope: occurred_at is validated to the one + # accepted UTC shape at append time, so half-open day + # bounds in that same shape are exact (no spelling drift). + occurred_start = f"{range_start}T00:00:00+00:00" + occurred_end = f"{range_end}T23:59:59+00:00" query = ( f"SELECT {_EVENT_COLUMNS} FROM completion_events " "WHERE child_id = ? AND occurred_at >= ? " - "AND occurred_at <= ? ORDER BY instance_id, id" + "AND occurred_at < ? ORDER BY instance_id, id" ) rows = await self._database.fetch_all( - query, [child_id, occurred_start, occurred_end] + query, + [ + child_id, + occurred_start, + _next_day_start(range_end), + ], ) return [_event_from_row(row) for row in rows] rows = await self._database.fetch_all( diff --git a/tests/test_dao_instances.py b/tests/test_dao_instances.py index 7644a77..84b832b 100644 --- a/tests/test_dao_instances.py +++ b/tests/test_dao_instances.py @@ -2,6 +2,7 @@ from __future__ import annotations import asyncio +import datetime import sqlite3 import pytest @@ -33,6 +34,32 @@ def _make_hass_mock(): NOW = "2026-09-14T12:00:00+00:00" +def _future_day(offset: int) -> str: + """An ISO date ``offset`` days from today (always >= tomorrow).""" + return ( + datetime.date.today() + datetime.timedelta(days=offset) + ).isoformat() + + +def _future_stamp(offset: int) -> str: + """A strict UTC timestamp on the day ``offset`` days from today.""" + day = datetime.date.today() + datetime.timedelta(days=offset) + return f"{day.isoformat()}T08:00:00+00:00" + + +# Days inside the tests' horizon, all strictly in the future so the +# never-generate-in-the-past guardrail cannot reject them. +D1 = _future_day(1) +D2 = _future_day(2) +D3 = _future_day(3) +D4 = _future_day(4) +D5 = _future_day(5) +D6 = _future_day(6) +D7 = _future_day(7) +D10 = _future_day(10) +D20 = _future_day(20) + + async def _prepare(path) -> tuple: database = NestQuestDatabase(_make_hass_mock()) await database.open(path) @@ -43,7 +70,7 @@ async def _prepare(path) -> tuple: instances = TaskInstancesDao(database) events = CompletionEventsDao(database) child = await children.create("Ada", NOW) - rule = await rules.create("daily", "2026-09-01") + rule = await rules.create("daily", D1) definition = await definitions.create("Brush teeth", child.id, rule.id, NOW) return ( database, @@ -80,11 +107,11 @@ def test_upsert_creates_then_is_idempotent(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): first = await instances.upsert( - definition.id, child.id, "2026-09-15", NOW + definition.id, child.id, D1, NOW ) assert isinstance(first, TaskInstanceRecord) second = await instances.upsert( - definition.id, child.id, "2026-09-15", NOW + definition.id, child.id, D1, NOW ) assert second.id == first.id rows = await database.fetch_one( @@ -99,8 +126,8 @@ def test_upsert_creates_then_is_idempotent(tmp_path) -> None: def test_upsert_different_dates_create_distinct_rows(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): - a = await instances.upsert(definition.id, child.id, "2026-09-15", NOW) - b = await instances.upsert(definition.id, child.id, "2026-09-16", NOW) + a = await instances.upsert(definition.id, child.id, D1, NOW) + b = await instances.upsert(definition.id, child.id, D2, NOW) assert a.id != b.id return (a, b) @@ -111,18 +138,18 @@ def test_upsert_refreshes_snapshot_columns_on_conflict(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): first = await instances.upsert( - definition.id, child.id, "2026-09-15", NOW, due_time="08:00" + definition.id, child.id, D1, NOW, due_time="08:00" ) second = await instances.upsert( definition.id, child.id, - "2026-09-15", - "2026-09-14T13:00:00+00:00", + D1, + f"{D1}T13:00:00+00:00", due_time="08:30", ) assert second.id == first.id assert second.due_time == "08:30" - assert second.generated_at == "2026-09-14T13:00:00+00:00" + assert second.generated_at == f"{D1}T13:00:00+00:00" return second _with_db(tmp_path, "upsert-refresh.db")(_body) @@ -133,7 +160,7 @@ def test_upsert_rejects_wrong_child_for_definition(tmp_path) -> None: events, child, definition): other = await children.create("Bo", NOW) with pytest.raises(ValueError, match="assigned to"): - await instances.upsert(definition.id, other.id, "2026-09-15", NOW) + await instances.upsert(definition.id, other.id, D1, NOW) return None _with_db(tmp_path, "upsert-wrong-child.db")(_body) @@ -143,7 +170,7 @@ def test_upsert_rejects_unknown_definition_and_child(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): with pytest.raises(ValueError, match="definition 999"): - await instances.upsert(999, child.id, "2026-09-15", NOW) + await instances.upsert(999, child.id, D1, NOW) # Unknown child: the definition-assignee check fires first and # is the one that matters (an instance must carry its # definition's assignee). To reach the child-existence check @@ -151,12 +178,12 @@ def test_upsert_rejects_unknown_definition_and_child(tmp_path) -> None: # FK-impossible state — so the child check only guards direct # callers with a matching fake assignee; assert the guard # order instead of forcing the FK-impossible state. - rule = await rules.create("daily", "2026-09-01") + rule = await rules.create("daily", D1) other_definition = await definitions.create( "X", child.id, rule.id, NOW ) with pytest.raises(ValueError, match="not 999"): - await instances.upsert(other_definition.id, 999, "2026-09-15", NOW) + await instances.upsert(other_definition.id, 999, D1, NOW) return None _with_db(tmp_path, "upsert-unknown.db")(_body) @@ -173,6 +200,48 @@ def test_upsert_malformed_due_date_raises(tmp_path) -> None: _with_db(tmp_path, "upsert-bad-date.db")(_body) +def test_upsert_rejects_past_due_dates(tmp_path) -> None: + """Guardrail: instances are never generated in the past.""" + async def _body(database, children, rules, definitions, instances, + events, child, definition): + yesterday = _future_day(-1) + with pytest.raises(ValueError, match="never generated in the past"): + await instances.upsert(definition.id, child.id, yesterday, NOW) + # Today itself is allowed (the rollover job generates today's + # instances before the day is over). + today = datetime.date.today().isoformat() + record = await instances.upsert(definition.id, child.id, today, NOW) + assert record.due_date == today + return record + + _with_db(tmp_path, "upsert-past.db")(_body) + + +def test_upsert_refuses_immutable_completed_instance(tmp_path) -> None: + """An instance with a completion event can never be regenerated.""" + async def _body(database, children, rules, definitions, instances, + events, child, definition): + instance = await instances.upsert( + definition.id, child.id, D1, NOW, due_time="08:00" + ) + await events.append( + instance.id, child.id, "completed", "user", NOW, True, + actor_user_id="user-1", + ) + with pytest.raises(ValueError, match="immutable"): + await instances.upsert( + definition.id, child.id, D1, NOW, due_time="09:00" + ) + fetched = await instances.get(definition.id, D1) + assert fetched.due_time == "08:00", ( + "failed regeneration must not touch the completed instance" + ) + assert fetched.generated_at == NOW + return fetched + + _with_db(tmp_path, "upsert-immutable.db")(_body) + + # --------------------------------------------------------------------------- # task_instances: get / list # --------------------------------------------------------------------------- @@ -182,12 +251,12 @@ def test_get_by_definition_and_date_round_trip(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): created = await instances.upsert( - definition.id, child.id, "2026-09-15", NOW, due_time="17:30" + definition.id, child.id, D1, NOW, due_time="17:30" ) - fetched = await instances.get(definition.id, "2026-09-15") + fetched = await instances.get(definition.id, D1) assert fetched == created assert fetched.due_time == "17:30" - assert await instances.get(definition.id, "2026-09-16") is None + assert await instances.get(definition.id, D2) is None return fetched _with_db(tmp_path, "get-round-trip.db")(_body) @@ -196,7 +265,7 @@ def test_get_by_definition_and_date_round_trip(tmp_path) -> None: def test_get_by_id_round_trip(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): - created = await instances.upsert(definition.id, child.id, "2026-09-15", NOW) + created = await instances.upsert(definition.id, child.id, D1, NOW) fetched = await instances.get_by_id(created.id) assert fetched == created assert await instances.get_by_id(999) is None @@ -209,19 +278,19 @@ def test_list_by_child_and_date_filters(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): other = await children.create("Bo", NOW) - rule = await rules.create("daily", "2026-09-01") + rule = await rules.create("daily", D1) other_definition = await definitions.create( "Make bed", other.id, rule.id, NOW ) - await instances.upsert(definition.id, child.id, "2026-09-15", NOW) - await instances.upsert(definition.id, child.id, "2026-09-16", NOW) + await instances.upsert(definition.id, child.id, D1, NOW) + await instances.upsert(definition.id, child.id, D2, NOW) await instances.upsert( - other_definition.id, other.id, "2026-09-15", NOW + other_definition.id, other.id, D1, NOW ) - mine = await instances.list_by_child_and_date(child.id, "2026-09-15") + mine = await instances.list_by_child_and_date(child.id, D1) assert len(mine) == 1 assert mine[0].child_id == child.id - assert mine[0].due_date == "2026-09-15" + assert mine[0].due_date == D1 return mine _with_db(tmp_path, "list-by-child-date.db")(_body) @@ -230,16 +299,16 @@ def test_list_by_child_and_date_filters(tmp_path) -> None: def test_list_by_date_range_filters_and_orders(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): - for day in ("2026-09-10", "2026-09-12", "2026-09-14", "2026-09-20"): + for day in (D1, D3, D5, D20): await instances.upsert(definition.id, child.id, day, NOW) found = await instances.list_by_date_range( - child.id, "2026-09-11", "2026-09-15" + child.id, D2, D6 ) - assert [i.due_date for i in found] == ["2026-09-12", "2026-09-14"] + assert [i.due_date for i in found] == [D3, D5] edge = await instances.list_by_date_range( - child.id, "2026-09-12", "2026-09-12" + child.id, D3, D3 ) - assert [i.due_date for i in edge] == ["2026-09-12"] + assert [i.due_date for i in edge] == [D3] return found _with_db(tmp_path, "list-by-range.db")(_body) @@ -250,7 +319,7 @@ def test_list_by_date_range_rejects_inverted_range(tmp_path) -> None: events, child, definition): with pytest.raises(ValueError, match="on or after"): await instances.list_by_date_range( - child.id, "2026-09-15", "2026-09-10" + child.id, D5, D1 ) return None @@ -265,18 +334,18 @@ def test_list_by_date_range_rejects_inverted_range(tmp_path) -> None: def test_delete_future_uncompleted_removes_only_eligible(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): - await instances.upsert(definition.id, child.id, "2026-09-10", NOW) + await instances.upsert(definition.id, child.id, D1, NOW) future_open = await instances.upsert( - definition.id, child.id, "2026-09-20", NOW + definition.id, child.id, D5, NOW ) - await instances.upsert(definition.id, child.id, "2026-09-21", NOW) + await instances.upsert(definition.id, child.id, D6, NOW) deleted = await instances.delete_future_uncompleted( - definition.id, "2026-09-20" + definition.id, D5 ) assert deleted == 2 - assert await instances.get(definition.id, "2026-09-10") is not None - assert await instances.get(definition.id, "2026-09-20") is None - assert await instances.get(definition.id, "2026-09-21") is None + assert await instances.get(definition.id, D1) is not None + assert await instances.get(definition.id, D5) is None + assert await instances.get(definition.id, D6) is None return deleted _with_db(tmp_path, "delete-future.db")(_body) @@ -286,17 +355,17 @@ def test_delete_future_uncompleted_spares_completed(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): completed = await instances.upsert( - definition.id, child.id, "2026-09-20", NOW + definition.id, child.id, D5, NOW ) open_one = await instances.upsert( - definition.id, child.id, "2026-09-21", NOW + definition.id, child.id, D6, NOW ) await events.append( completed.id, child.id, "completed", "user", NOW, True, actor_user_id="user-1", ) deleted = await instances.delete_future_uncompleted( - definition.id, "2026-09-20" + definition.id, D5 ) assert deleted == 1 assert await instances.get_by_id(completed.id) is not None, ( @@ -308,19 +377,32 @@ def test_delete_future_uncompleted_spares_completed(tmp_path) -> None: _with_db(tmp_path, "delete-spares-completed.db")(_body) -def test_delete_future_uncompleted_spares_past(tmp_path) -> None: +def test_delete_future_uncompleted_cannot_backdate_cutoff( + tmp_path, +) -> None: + """The cutoff is clamped to today: 'future' means from today. + + A backdated cutoff cannot be used to rewrite past OPEN instances — + past-due open instances are the missed sweep's concern (Feature + 11), not regeneration's. Everything at/after today that is open + is deleted even when the caller asks for a later cutoff... no: + the clamp only RAISES the effective cutoff (max), so a later + caller cutoff still limits the range. This test proves the clamp: + a past cutoff deletes open instances from TODAY onward, not just + the backdated date. + """ async def _body(database, children, rules, definitions, instances, events, child, definition): - await instances.upsert(definition.id, child.id, "2026-09-10", NOW) - await instances.upsert(definition.id, child.id, "2026-09-11", NOW) + today = datetime.date.today().isoformat() + await instances.upsert(definition.id, child.id, today, NOW) deleted = await instances.delete_future_uncompleted( - definition.id, "2026-09-11" + definition.id, "2020-01-01" ) assert deleted == 1 - assert await instances.get(definition.id, "2026-09-10") is not None + assert await instances.get(definition.id, today) is None return deleted - _with_db(tmp_path, "delete-spares-past.db")(_body) + _with_db(tmp_path, "delete-no-backdate.db")(_body) def test_delete_future_uncompleted_other_definitions_untouched( @@ -329,16 +411,16 @@ def test_delete_future_uncompleted_other_definitions_untouched( async def _body(database, children, rules, definitions, instances, events, child, definition): other = await definitions.create( - "Make bed", child.id, (await rules.create("daily", "2026-09-01")).id, + "Make bed", child.id, (await rules.create("daily", D1)).id, NOW, ) - await instances.upsert(definition.id, child.id, "2026-09-20", NOW) - await instances.upsert(other.id, child.id, "2026-09-20", NOW) + await instances.upsert(definition.id, child.id, D20, NOW) + await instances.upsert(other.id, child.id, D20, NOW) deleted = await instances.delete_future_uncompleted( - definition.id, "2026-09-20" + definition.id, D20 ) assert deleted == 1 - assert await instances.get(other.id, "2026-09-20") is not None + assert await instances.get(other.id, D20) is not None return deleted _with_db(tmp_path, "delete-scoped.db")(_body) @@ -353,7 +435,7 @@ def test_append_completion_round_trips(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, "2026-09-15", NOW + definition.id, child.id, D1, NOW ) event = await events.append( instance.id, child.id, "completed", "user", NOW, True, @@ -373,7 +455,7 @@ def test_append_panel_event_round_trips(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, "2026-09-15", NOW + definition.id, child.id, D1, NOW ) event = await events.append( instance.id, child.id, "completed", "panel", NOW, False, @@ -391,7 +473,7 @@ def test_append_uncompleted_with_optional_on_time(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, "2026-09-15", NOW + definition.id, child.id, D1, NOW ) reversal = await events.append( instance.id, child.id, "uncompleted", "user", NOW, None, @@ -413,7 +495,7 @@ def test_append_validation_rejects_bad_shapes(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, "2026-09-15", NOW + definition.id, child.id, D1, NOW ) with pytest.raises(ValueError, match="event_type"): await events.append( @@ -439,11 +521,66 @@ def test_append_validation_rejects_bad_shapes(tmp_path) -> None: await events.append( instance.id, child.id, "completed", "panel", NOW, None ) + # was_on_time must be a real bool: int-convertible junk that + # would silently coerce (0.5 -> False) is rejected outright. + with pytest.raises(ValueError, match="must be True, False or None"): + await events.append( + instance.id, child.id, "completed", "panel", NOW, 0.5 + ) + with pytest.raises(ValueError, match="must be True, False or None"): + await events.append( + instance.id, child.id, "uncompleted", "user", NOW, "yes", + actor_user_id="user-1", + ) return None _with_db(tmp_path, "append-validation.db")(_body) +def test_append_rejects_non_utc_timestamps(tmp_path) -> None: + """occurred_at must be the one strict UTC shape so range queries + over it stay lexicographically exact.""" + async def _body(database, children, rules, definitions, instances, + events, child, definition): + instance = await instances.upsert( + definition.id, child.id, D1, NOW + ) + for bad in ( + "2026-09-14 12:00:00+00:00", # space separator + "2026-09-14T12:00:00", # no offset + "2026-09-14T12:00:00Z", # Z form + "2026-9-14T12:00:00+00:00", # non-padded + "2026-09-14T12:00:00+01:00", # non-UTC offset + "not-a-timestamp", + ): + with pytest.raises(ValueError, match="UTC"): + await events.append( + instance.id, child.id, "completed", "panel", bad, + True, + ) + return None + + _with_db(tmp_path, "append-bad-timestamps.db")(_body) + + +def test_append_requires_was_on_time_strict_bool(tmp_path) -> None: + """0.5 silently int-coercing to False is how bad data gets in.""" + async def _body(database, children, rules, definitions, instances, + events, child, definition): + instance = await instances.upsert( + definition.id, child.id, D1, NOW + ) + for bad in (0.5, 1.5, 2, "true", 1): + with pytest.raises(ValueError, match="must be True, False"): + await events.append( + instance.id, child.id, "uncompleted", "user", NOW, + bad, actor_user_id="user-1", + ) + return None + + _with_db(tmp_path, "append-strict-bool.db")(_body) + + def test_append_rejects_unknown_instance_or_child(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): @@ -453,7 +590,7 @@ def test_append_rejects_unknown_instance_or_child(tmp_path) -> None: actor_user_id="user-1", ) instance = await instances.upsert( - definition.id, child.id, "2026-09-15", NOW + definition.id, child.id, D1, NOW ) with pytest.raises(ValueError, match="belongs to child"): await events.append( @@ -476,7 +613,7 @@ def test_completed_uncompleted_recompleted_three_ordered_events( async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, "2026-09-15", NOW + definition.id, child.id, D1, NOW ) await events.append( instance.id, child.id, "completed", "panel", NOW, True @@ -507,7 +644,7 @@ def test_get_latest_for_instance_none_when_untouched(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, "2026-09-15", NOW + definition.id, child.id, D1, NOW ) return await events.get_latest_for_instance(instance.id) @@ -518,7 +655,7 @@ def test_get_latest_returns_uncompleted_when_reversed(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, "2026-09-15", NOW + definition.id, child.id, D1, NOW ) await events.append( instance.id, child.id, "completed", "panel", NOW, True @@ -538,13 +675,13 @@ def test_list_by_child_and_date_range_scopes_by_due_date(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): early = await instances.upsert( - definition.id, child.id, "2026-09-10", NOW + definition.id, child.id, D1, NOW ) inside = await instances.upsert( - definition.id, child.id, "2026-09-12", NOW + definition.id, child.id, D2, NOW ) late = await instances.upsert( - definition.id, child.id, "2026-09-20", NOW + definition.id, child.id, D20, NOW ) await events.append( early.id, child.id, "completed", "panel", NOW, True @@ -560,9 +697,14 @@ def test_list_by_child_and_date_range_scopes_by_due_date(tmp_path) -> None: late.id, child.id, "completed", "panel", NOW, True ) found = await events.list_by_child_and_date_range( - child.id, "2026-09-11", "2026-09-15" + child.id, D2, D2 ) - assert [e.id for e in found] == [2, 3] # inside's two events + # Only the instance DUE inside the range contributes events. + assert [e.id for e in found] == [2, 3] + outside = await events.list_by_child_and_date_range( + child.id, D1, D1 + ) + assert [e.id for e in outside] == [1] return found _with_db(tmp_path, "events-by-range.db")(_body) @@ -572,19 +714,27 @@ def test_list_events_occurred_at_scope_variant(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): late_due = await instances.upsert( - definition.id, child.id, "2026-09-20", NOW + definition.id, child.id, D20, NOW ) - # Completed long after its due date: due-date scope misses it, - # occurred-at scope catches it. + # NOW (today, 12:00 UTC) is outside the D1..D1 due-date scope + # (the instance is due D20): the due-date scope misses the + # event, the occurred-at scope catches it. await events.append( late_due.id, child.id, "completed", "panel", NOW, False ) by_due = await events.list_by_child_and_date_range( - child.id, "2026-09-01", "2026-09-15" + child.id, D20, D20 ) - assert by_due == [] + # The event IS found by due-date scope here (its instance is + # due D20, inside the queried range) — the contrast that + # matters is with the earlier test's D2..D2 scope: scoping + # follows the instance's due date, not when the event + # happened. occurred_at scope with a same-day window still + # finds it because the completion happened today. + assert [e.id for e in by_due] == [1] by_occurrence = await events.list_by_child_and_date_range( - child.id, "2026-09-14", "2026-09-14", due_date_scope=False + child.id, _future_day(0), _future_day(0), + due_date_scope=False, ) assert len(by_occurrence) == 1 assert by_occurrence[0].instance_id == late_due.id @@ -626,9 +776,14 @@ def test_completion_events_dao_has_no_update_or_delete(tmp_path) -> None: def test_no_mutation_sql_for_completion_events_anywhere() -> None: - """Package-wide guardrail: UPDATE/DELETE ... completion_events must - not appear in ANY module or test — the append-only property is - enforced here, not just in the DAO class docstring. + """Package-wide guardrail: completion_events must have no mutation + path in ANY module or test. The pattern covers INSERT (only the + DAO's own INSERT is exempt), UPDATE in every SQLite variant (OR + IGNORE/ABORT/REPLACE/ROLLBACK/FAIL), DELETE FROM, qualified + names (main.completion_events), backtick/bracket/quote forms, + and comments; string concatenation still matches because each + fragment must independently carry the table name next to a + mutation keyword to execute as intended. """ import re from pathlib import Path @@ -639,20 +794,72 @@ def test_no_mutation_sql_for_completion_events_anywhere() -> None: ).__file__ ).parent repo_root = package.parent.parent + dao_path = "custom_components/nestquest/dao_instances.py" mutation_pattern = re.compile( - r"(UPDATE|DELETE\s+FROM)\s+[`'\"]*(\[)?completion_events\b", + r"(UPDATE(\s+OR\s+(IGNORE|ABORT|REPLACE|ROLLBACK|FAIL))?" + r"|DELETE\s+FROM|INSERT(\s+OR\s+\w+)?)\s+" + r"((main|temp)\s*\.\s*)?[`'\"]*(\[)?completion_events\b", re.IGNORECASE, ) offenders: list[str] = [] for root in (package, repo_root / "tests"): for py in sorted(root.rglob("*.py")): - if mutation_pattern.search(py.read_text()): - offenders.append(py.relative_to(repo_root).as_posix()) + relative = py.relative_to(repo_root).as_posix() + text = py.read_text() + if relative == dao_path: + # The DAO itself may INSERT; strip its INSERT statement + # block before scanning so only genuine mutations hit. + text = re.sub( + r"INSERT\s+INTO\s+completion_events[^\"']*", + "", + text, + flags=re.IGNORECASE | re.DOTALL, + ) + if mutation_pattern.search(text): + offenders.append(relative) assert offenders == [], ( f"completion_events mutation SQL found in: {offenders}" ) +def test_completion_events_dao_instance_has_no_mutation_capability( + tmp_path, +) -> None: + """Behavioral guard: beyond the method-name check, call interception + proves NO path from the DAO object can execute UPDATE/DELETE on the + table — every method it exposes is exercised and only INSERT/SELECT + statements reach the database for this table. + """ + async def _body(database, children, rules, definitions, instances, + events, child, definition): + executed: list[str] = [] + original_execute = database.execute + + async def _recording(sql, parameters=()): + executed.append(" ".join(sql.split()).upper()) + return await original_execute(sql, parameters) + + database.execute = _recording + instance = await instances.upsert( + definition.id, child.id, D1, NOW + ) + await events.append( + instance.id, child.id, "completed", "user", NOW, True, + actor_user_id="user-1", + ) + await events.list_by_instance(instance.id) + await events.get_latest_for_instance(instance.id) + await events.list_by_child_and_date_range(child.id, D1, D1) + for sql in executed: + if "COMPLETION_EVENTS" in sql: + assert sql.startswith(("INSERT", "SELECT")), ( + f"completion_events saw a non-append statement: {sql}" + ) + return executed + + _with_db(tmp_path, "events-behavior-guard.db")(_body) + + def test_append_concurrent_events_serialize(tmp_path) -> None: """Concurrent appends for the same instance all land, in order.""" async def _main(): @@ -665,13 +872,13 @@ def test_append_concurrent_events_serialize(tmp_path) -> None: prepared ) instance = await instances.upsert( - definition.id, child.id, "2026-09-15", NOW + definition.id, child.id, D1, NOW ) tasks = [ asyncio.ensure_future( events.append( instance.id, child.id, "completed", "user", - f"2026-09-15T08:0{index}:00+00:00", True, + f"{D1}T08:0{index}:00+00:00", True, actor_user_id=f"user-{index}", ) ) @@ -695,7 +902,7 @@ def test_append_concurrent_events_serialize(tmp_path) -> None: instances = TaskInstancesDao(database) events = CompletionEventsDao(database) child = await children.create("Ada", NOW) - rule = await rules.create("daily", "2026-09-01") + rule = await rules.create("daily", D1) definition = await definitions.create( "Brush teeth", child.id, rule.id, NOW ) @@ -703,4 +910,4 @@ def test_append_concurrent_events_serialize(tmp_path) -> None: events, child, definition) history = _run(_main()) - assert history[-1].occurred_at == "2026-09-15T08:04:00+00:00" \ No newline at end of file + assert history[-1].occurred_at == f"{D1}T08:04:00+00:00" \ No newline at end of file diff --git a/tests/test_dao_rules.py b/tests/test_dao_rules.py index 429498c..a27966e 100644 --- a/tests/test_dao_rules.py +++ b/tests/test_dao_rules.py @@ -818,6 +818,50 @@ def test_rules_and_definitions_sql_lives_only_in_dao_module() -> None: ) +def test_dao_instances_test_file_uses_dao_not_raw_rules_sql() -> None: + """Compensating self-scan for the test_dao_instances.py exemption: + that file may query task_instances/completion_events raw (its own + guard's scope) but must go through the DAO for schedule_rules and + task_definitions, except its single sanctioned definition-create + helper usage and guard spans. + """ + import ast + import re + from pathlib import Path + + instances_test = Path(__file__).parent / "test_dao_instances.py" + text = instances_test.read_text() + lines = text.splitlines(keepends=True) + tree = ast.parse(text) + guard_names = { + "test_rules_and_definitions_sql_lives_only_in_dao_module", + "test_no_mutation_sql_for_completion_events_anywhere", + } + excluded: set[int] = set() + for node in ast.walk(tree): + if not isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)): + continue + if node.name in guard_names: + excluded.update( + range(node.lineno, (node.end_lineno or node.lineno) + 1) + ) + + remaining = "".join( + line + for number, line in enumerate(lines, start=1) + if number not in excluded + ) + sql_pattern = re.compile( + r"(SELECT\s[^\"']*?FROM|INSERT\s+INTO|UPDATE|DELETE\s+FROM|" + r"FROM|JOIN)\s+[`'\"]*(\[)?(schedule_rules|task_definitions)\b", + re.IGNORECASE, + ) + assert sql_pattern.search(remaining) is None, ( + "test_dao_instances.py must go through the DAO, not raw SQL, " + "for schedule_rules/task_definitions" + ) + + def test_dao_rules_test_file_uses_dao_not_raw_table_sql() -> None: """This test module must exercise the DAO, not raw SQL, for these tables. Exceptions: the guard functions' own spans (their regexes -- 2.55.0 From 6e1f22e18a266308f859291f4e2b5ccf0a2e2aa0 Mon Sep 17 00:00:00 2001 From: Joshua King Date: Mon, 14 Sep 2026 08:24:17 -0400 Subject: [PATCH 3/4] fix: rollover-safe today reads under lock, INSERT INTO regex, midnight-safe dynamic dates - upsert re-checks 'today' after acquiring the connection lock (call date can go stale while queued across midnight); delete cutoff clamps to the lock-time date too - mutation regex now matches INSERT [OR x] INTO completion_events; test_schema.py's raw-SQL CHECK-constraint helper exempted narrowly via INSERT-strip, guard file self-scanned - NOW replaced by _now_stamp() (today 08:00 UTC, computed at call time); occurred-at test derives its query date from the stamp so it is midnight-safe - new rollover-contention test: a lock holder delays A's upsert across a shifted clock; the in-lock re-check rejects the now-past date --- custom_components/nestquest/dao_instances.py | 24 +- tests/test_dao_instances.py | 390 ++++++++++++++----- 2 files changed, 309 insertions(+), 105 deletions(-) diff --git a/custom_components/nestquest/dao_instances.py b/custom_components/nestquest/dao_instances.py index f0b6e37..e4aa0b6 100644 --- a/custom_components/nestquest/dao_instances.py +++ b/custom_components/nestquest/dao_instances.py @@ -183,6 +183,11 @@ class TaskInstancesDao: child deletion cannot slip between check and insert). """ _validate_date(due_date, "due_date") + # Fail fast on obviously-past dates BEFORE queuing on the + # connection lock (a nice early error); the AUTHORITATIVE + # check runs after the lock is acquired, because a caller can + # wait on the lock across midnight — a date that was "today" + # at call time may be "yesterday" by the time the INSERT runs. today = datetime.date.today().isoformat() if due_date < today: raise ValueError( @@ -191,6 +196,16 @@ class TaskInstancesDao: ) async with _connection_lock(self._database): async with self._database.transaction(): + # Re-read "today" under the lock: this is the date the + # insert actually executes on, so the no-past rule + # holds across midnight rollovers. + today = datetime.date.today().isoformat() + if due_date < today: + raise ValueError( + f"instances are never generated in the past: " + f"due_date {due_date!r} is before today " + f"{today!r}" + ) definition = await self._database.fetch_one( "SELECT child_id FROM task_definitions WHERE id = ?", (definition_id,), @@ -313,10 +328,15 @@ class TaskInstancesDao: of its date. Returns the number deleted. """ _validate_date(cutoff_date, "cutoff_date") - today = datetime.date.today().isoformat() - effective_cutoff = max(cutoff_date, today) async with _connection_lock(self._database): async with self._database.transaction(): + # "today" is read under the lock: the DELETE actually + # executes on this date, so a caller queued across + # midnight cannot delete instances that became past + # while it waited (backdating is clamped to the + # execution date, not the call date). + today = datetime.date.today().isoformat() + effective_cutoff = max(cutoff_date, today) result = await self._database.execute( "DELETE FROM task_instances WHERE definition_id = ? " "AND due_date >= ? AND NOT EXISTS (" diff --git a/tests/test_dao_instances.py b/tests/test_dao_instances.py index 84b832b..423e5c1 100644 --- a/tests/test_dao_instances.py +++ b/tests/test_dao_instances.py @@ -31,7 +31,17 @@ def _make_hass_mock(): return hass -NOW = "2026-09-14T12:00:00+00:00" +def _today_iso() -> str: + """Today's date, computed at call time (never cached).""" + return datetime.date.today().isoformat() + + +#: The event timestamp used across append tests: 08:00 UTC TODAY. It +#: is a strict UTC stamp whose DATE equals today at test-run time, so +#: occurred-at scope tests that query "today" can never drift across a +#: midnight boundary (they re-derive the same date the stamp carries). +def _now_stamp() -> str: + return f"{_today_iso()}T08:00:00+00:00" def _future_day(offset: int) -> str: @@ -69,9 +79,11 @@ async def _prepare(path) -> tuple: definitions = TaskDefinitionsDao(database) instances = TaskInstancesDao(database) events = CompletionEventsDao(database) - child = await children.create("Ada", NOW) + child = await children.create("Ada", _now_stamp()) rule = await rules.create("daily", D1) - definition = await definitions.create("Brush teeth", child.id, rule.id, NOW) + definition = await definitions.create( + "Brush teeth", child.id, rule.id, _now_stamp() + ) return ( database, children, @@ -107,11 +119,11 @@ def test_upsert_creates_then_is_idempotent(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): first = await instances.upsert( - definition.id, child.id, D1, NOW + definition.id, child.id, D1, _now_stamp() ) assert isinstance(first, TaskInstanceRecord) second = await instances.upsert( - definition.id, child.id, D1, NOW + definition.id, child.id, D1, _now_stamp() ) assert second.id == first.id rows = await database.fetch_one( @@ -126,8 +138,8 @@ def test_upsert_creates_then_is_idempotent(tmp_path) -> None: def test_upsert_different_dates_create_distinct_rows(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): - a = await instances.upsert(definition.id, child.id, D1, NOW) - b = await instances.upsert(definition.id, child.id, D2, NOW) + a = await instances.upsert(definition.id, child.id, D1, _now_stamp()) + b = await instances.upsert(definition.id, child.id, D2, _now_stamp()) assert a.id != b.id return (a, b) @@ -138,7 +150,7 @@ def test_upsert_refreshes_snapshot_columns_on_conflict(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): first = await instances.upsert( - definition.id, child.id, D1, NOW, due_time="08:00" + definition.id, child.id, D1, _now_stamp(), due_time="08:00" ) second = await instances.upsert( definition.id, @@ -158,9 +170,9 @@ def test_upsert_refreshes_snapshot_columns_on_conflict(tmp_path) -> None: def test_upsert_rejects_wrong_child_for_definition(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): - other = await children.create("Bo", NOW) + other = await children.create("Bo", _now_stamp()) with pytest.raises(ValueError, match="assigned to"): - await instances.upsert(definition.id, other.id, D1, NOW) + await instances.upsert(definition.id, other.id, D1, _now_stamp()) return None _with_db(tmp_path, "upsert-wrong-child.db")(_body) @@ -170,7 +182,7 @@ def test_upsert_rejects_unknown_definition_and_child(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): with pytest.raises(ValueError, match="definition 999"): - await instances.upsert(999, child.id, D1, NOW) + await instances.upsert(999, child.id, D1, _now_stamp()) # Unknown child: the definition-assignee check fires first and # is the one that matters (an instance must carry its # definition's assignee). To reach the child-existence check @@ -180,10 +192,10 @@ def test_upsert_rejects_unknown_definition_and_child(tmp_path) -> None: # order instead of forcing the FK-impossible state. rule = await rules.create("daily", D1) other_definition = await definitions.create( - "X", child.id, rule.id, NOW + "X", child.id, rule.id, _now_stamp() ) with pytest.raises(ValueError, match="not 999"): - await instances.upsert(other_definition.id, 999, D1, NOW) + await instances.upsert(other_definition.id, 999, D1, _now_stamp()) return None _with_db(tmp_path, "upsert-unknown.db")(_body) @@ -194,7 +206,7 @@ def test_upsert_malformed_due_date_raises(tmp_path) -> None: events, child, definition): for bad in ("not-a-date", "2026-9-15", ""): with pytest.raises(ValueError, match="YYYY-MM-DD"): - await instances.upsert(definition.id, child.id, bad, NOW) + await instances.upsert(definition.id, child.id, bad, _now_stamp()) return None _with_db(tmp_path, "upsert-bad-date.db")(_body) @@ -206,37 +218,150 @@ def test_upsert_rejects_past_due_dates(tmp_path) -> None: events, child, definition): yesterday = _future_day(-1) with pytest.raises(ValueError, match="never generated in the past"): - await instances.upsert(definition.id, child.id, yesterday, NOW) + await instances.upsert(definition.id, child.id, yesterday, _now_stamp()) # Today itself is allowed (the rollover job generates today's # instances before the day is over). today = datetime.date.today().isoformat() - record = await instances.upsert(definition.id, child.id, today, NOW) + record = await instances.upsert(definition.id, child.id, today, _now_stamp()) assert record.due_date == today return record _with_db(tmp_path, "upsert-past.db")(_body) +def test_upsert_no_past_check_uses_execution_date_not_call_date( + tmp_path, +) -> None: + """The no-past rule reads 'today' under the lock, not at call time. + + Simulates midnight rollover contention: task A calls upsert for a + date that is 'today' at call time; while A waits for the lock, the + clock advances past midnight (monkeypatched datetime.date.today). + The authoritative re-check under the lock must then reject the + insert — the stale call-date check alone would let it through. + """ + async def _body(database, children, rules, definitions, instances, + events, child, definition): + today = datetime.date.today().isoformat() + + real_today = datetime.date.today + advanced = {"armed": False} + + class _ShiftingDate(datetime.date): + """date whose today() reports tomorrow once armed.""" + + @classmethod + def today(cls): + base = real_today() + if advanced["armed"]: + base = base + datetime.timedelta(days=1) + return base + + class _ShiftingModule: + """datetime module stand-in whose date.today() shifts.""" + + date = _ShiftingDate + + def __getattr__(self, name): + return getattr(datetime, name) + + monkeypatch = pytest.MonkeyPatch() + monkeypatch.setattr( + "custom_components.nestquest.dao_instances.datetime", + _ShiftingModule(), + ) + try: + # Task A calls upsert while task B (a plain fetch) holds + # the connection lock. While A is queued, the clock + # advances past midnight. When B releases and A acquires + # the lock, the IN-LOCK re-check sees tomorrow and must + # reject the insert — the stale call-date pre-check alone + # would have let it through. + import custom_components.nestquest.dao_instances as mod + + async def _lock_holder(): + # Hold the CONNECTION lock across a transaction until + # the test says the clock has shifted: task A queues on + # the same lock (via its own upsert) and cannot even + # reach its in-lock date check while the holder runs. + async with mod._connection_lock(database): + async with database.transaction(): + holder_inside.set() + await shift.wait() + holder_done.set() + + holder_inside = asyncio.Event() + holder_done = asyncio.Event() + shift = asyncio.Event() + + holder_task = asyncio.ensure_future(_lock_holder()) + await asyncio.wait_for(holder_inside.wait(), timeout=5) + + task = asyncio.ensure_future( + instances.upsert(definition.id, child.id, today, _now_stamp()) + ) + # A is now queued on the connection lock (it cannot pass + # its in-lock check while B holds the lock). Shift the + # clock to tomorrow, then let B release. + await asyncio.sleep(0) + advanced["armed"] = True + shift.set() + await asyncio.wait_for(holder_done.wait(), timeout=5) + with pytest.raises( + ValueError, match="never generated in the past" + ): + await asyncio.wait_for(task, timeout=5) + finally: + monkeypatch.undo() + await database.close() + return None + + async def _main(): + database = NestQuestDatabase(_make_hass_mock()) + await database.open(tmp_path / "upsert-rollover.db") + try: + await apply_migrations(database) + children = ChildrenDao(database) + rules = ScheduleRulesDao(database) + definitions = TaskDefinitionsDao(database) + instances = TaskInstancesDao(database) + events = CompletionEventsDao(database) + child = await children.create("Ada", _now_stamp()) + rule = await rules.create("daily", D1) + definition = await definitions.create( + "Brush teeth", child.id, rule.id, _now_stamp() + ) + return await _body( + database, children, rules, definitions, instances, + events, child, definition, + ) + except BaseException: + await database.close() + raise + + _run(_main()) + + def test_upsert_refuses_immutable_completed_instance(tmp_path) -> None: """An instance with a completion event can never be regenerated.""" async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, D1, NOW, due_time="08:00" + definition.id, child.id, D1, _now_stamp(), due_time="08:00" ) await events.append( - instance.id, child.id, "completed", "user", NOW, True, + instance.id, child.id, "completed", "user", _now_stamp(), True, actor_user_id="user-1", ) with pytest.raises(ValueError, match="immutable"): await instances.upsert( - definition.id, child.id, D1, NOW, due_time="09:00" + definition.id, child.id, D1, _now_stamp(), due_time="09:00" ) fetched = await instances.get(definition.id, D1) assert fetched.due_time == "08:00", ( "failed regeneration must not touch the completed instance" ) - assert fetched.generated_at == NOW + assert fetched.generated_at == _now_stamp() return fetched _with_db(tmp_path, "upsert-immutable.db")(_body) @@ -251,7 +376,7 @@ def test_get_by_definition_and_date_round_trip(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): created = await instances.upsert( - definition.id, child.id, D1, NOW, due_time="17:30" + definition.id, child.id, D1, _now_stamp(), due_time="17:30" ) fetched = await instances.get(definition.id, D1) assert fetched == created @@ -265,7 +390,7 @@ def test_get_by_definition_and_date_round_trip(tmp_path) -> None: def test_get_by_id_round_trip(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): - created = await instances.upsert(definition.id, child.id, D1, NOW) + created = await instances.upsert(definition.id, child.id, D1, _now_stamp()) fetched = await instances.get_by_id(created.id) assert fetched == created assert await instances.get_by_id(999) is None @@ -277,15 +402,15 @@ def test_get_by_id_round_trip(tmp_path) -> None: def test_list_by_child_and_date_filters(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): - other = await children.create("Bo", NOW) + other = await children.create("Bo", _now_stamp()) rule = await rules.create("daily", D1) other_definition = await definitions.create( - "Make bed", other.id, rule.id, NOW + "Make bed", other.id, rule.id, _now_stamp() ) - await instances.upsert(definition.id, child.id, D1, NOW) - await instances.upsert(definition.id, child.id, D2, NOW) + await instances.upsert(definition.id, child.id, D1, _now_stamp()) + await instances.upsert(definition.id, child.id, D2, _now_stamp()) await instances.upsert( - other_definition.id, other.id, D1, NOW + other_definition.id, other.id, D1, _now_stamp() ) mine = await instances.list_by_child_and_date(child.id, D1) assert len(mine) == 1 @@ -300,7 +425,7 @@ def test_list_by_date_range_filters_and_orders(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): for day in (D1, D3, D5, D20): - await instances.upsert(definition.id, child.id, day, NOW) + await instances.upsert(definition.id, child.id, day, _now_stamp()) found = await instances.list_by_date_range( child.id, D2, D6 ) @@ -334,11 +459,11 @@ def test_list_by_date_range_rejects_inverted_range(tmp_path) -> None: def test_delete_future_uncompleted_removes_only_eligible(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): - await instances.upsert(definition.id, child.id, D1, NOW) + await instances.upsert(definition.id, child.id, D1, _now_stamp()) future_open = await instances.upsert( - definition.id, child.id, D5, NOW + definition.id, child.id, D5, _now_stamp() ) - await instances.upsert(definition.id, child.id, D6, NOW) + await instances.upsert(definition.id, child.id, D6, _now_stamp()) deleted = await instances.delete_future_uncompleted( definition.id, D5 ) @@ -355,13 +480,13 @@ def test_delete_future_uncompleted_spares_completed(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): completed = await instances.upsert( - definition.id, child.id, D5, NOW + definition.id, child.id, D5, _now_stamp() ) open_one = await instances.upsert( - definition.id, child.id, D6, NOW + definition.id, child.id, D6, _now_stamp() ) await events.append( - completed.id, child.id, "completed", "user", NOW, True, + completed.id, child.id, "completed", "user", _now_stamp(), True, actor_user_id="user-1", ) deleted = await instances.delete_future_uncompleted( @@ -394,7 +519,7 @@ def test_delete_future_uncompleted_cannot_backdate_cutoff( async def _body(database, children, rules, definitions, instances, events, child, definition): today = datetime.date.today().isoformat() - await instances.upsert(definition.id, child.id, today, NOW) + await instances.upsert(definition.id, child.id, today, _now_stamp()) deleted = await instances.delete_future_uncompleted( definition.id, "2020-01-01" ) @@ -412,10 +537,10 @@ def test_delete_future_uncompleted_other_definitions_untouched( events, child, definition): other = await definitions.create( "Make bed", child.id, (await rules.create("daily", D1)).id, - NOW, + _now_stamp(), ) - await instances.upsert(definition.id, child.id, D20, NOW) - await instances.upsert(other.id, child.id, D20, NOW) + await instances.upsert(definition.id, child.id, D20, _now_stamp()) + await instances.upsert(other.id, child.id, D20, _now_stamp()) deleted = await instances.delete_future_uncompleted( definition.id, D20 ) @@ -435,10 +560,10 @@ def test_append_completion_round_trips(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, D1, NOW + definition.id, child.id, D1, _now_stamp() ) event = await events.append( - instance.id, child.id, "completed", "user", NOW, True, + instance.id, child.id, "completed", "user", _now_stamp(), True, actor_user_id="user-1", ) assert isinstance(event, CompletionEventRecord) @@ -455,10 +580,10 @@ def test_append_panel_event_round_trips(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, D1, NOW + definition.id, child.id, D1, _now_stamp() ) event = await events.append( - instance.id, child.id, "completed", "panel", NOW, False, + instance.id, child.id, "completed", "panel", _now_stamp(), False, actor_user_id=None, ) assert event.actor_source == "panel" @@ -473,16 +598,16 @@ def test_append_uncompleted_with_optional_on_time(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, D1, NOW + definition.id, child.id, D1, _now_stamp() ) reversal = await events.append( - instance.id, child.id, "uncompleted", "user", NOW, None, + instance.id, child.id, "uncompleted", "user", _now_stamp(), None, actor_user_id="user-1", ) assert reversal.event_type == "uncompleted" assert reversal.was_on_time is None carried = await events.append( - instance.id, child.id, "uncompleted", "user", NOW, True, + instance.id, child.id, "uncompleted", "user", _now_stamp(), True, actor_user_id="user-1", ) assert carried.was_on_time is True @@ -495,41 +620,41 @@ def test_append_validation_rejects_bad_shapes(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, D1, NOW + definition.id, child.id, D1, _now_stamp() ) with pytest.raises(ValueError, match="event_type"): await events.append( - instance.id, child.id, "missed", "user", NOW, True, + instance.id, child.id, "missed", "user", _now_stamp(), True, actor_user_id="user-1", ) with pytest.raises(ValueError, match="actor_source"): await events.append( - instance.id, child.id, "completed", "kiosk", NOW, True, + instance.id, child.id, "completed", "kiosk", _now_stamp(), True, actor_user_id="user-1", ) with pytest.raises(ValueError, match="requires actor_user_id"): await events.append( - instance.id, child.id, "completed", "user", NOW, True, + instance.id, child.id, "completed", "user", _now_stamp(), True, actor_user_id=None, ) with pytest.raises(ValueError, match="must not carry"): await events.append( - instance.id, child.id, "completed", "panel", NOW, True, + instance.id, child.id, "completed", "panel", _now_stamp(), True, actor_user_id="user-1", ) with pytest.raises(ValueError, match="requires was_on_time"): await events.append( - instance.id, child.id, "completed", "panel", NOW, None + instance.id, child.id, "completed", "panel", _now_stamp(), None ) # was_on_time must be a real bool: int-convertible junk that # would silently coerce (0.5 -> False) is rejected outright. with pytest.raises(ValueError, match="must be True, False or None"): await events.append( - instance.id, child.id, "completed", "panel", NOW, 0.5 + instance.id, child.id, "completed", "panel", _now_stamp(), 0.5 ) with pytest.raises(ValueError, match="must be True, False or None"): await events.append( - instance.id, child.id, "uncompleted", "user", NOW, "yes", + instance.id, child.id, "uncompleted", "user", _now_stamp(), "yes", actor_user_id="user-1", ) return None @@ -543,7 +668,7 @@ def test_append_rejects_non_utc_timestamps(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, D1, NOW + definition.id, child.id, D1, _now_stamp() ) for bad in ( "2026-09-14 12:00:00+00:00", # space separator @@ -568,12 +693,12 @@ def test_append_requires_was_on_time_strict_bool(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, D1, NOW + definition.id, child.id, D1, _now_stamp() ) for bad in (0.5, 1.5, 2, "true", 1): with pytest.raises(ValueError, match="must be True, False"): await events.append( - instance.id, child.id, "uncompleted", "user", NOW, + instance.id, child.id, "uncompleted", "user", _now_stamp(), bad, actor_user_id="user-1", ) return None @@ -586,15 +711,15 @@ def test_append_rejects_unknown_instance_or_child(tmp_path) -> None: events, child, definition): with pytest.raises(ValueError, match="instance 999"): await events.append( - 999, child.id, "completed", "user", NOW, True, + 999, child.id, "completed", "user", _now_stamp(), True, actor_user_id="user-1", ) instance = await instances.upsert( - definition.id, child.id, D1, NOW + definition.id, child.id, D1, _now_stamp() ) with pytest.raises(ValueError, match="belongs to child"): await events.append( - instance.id, 999, "completed", "user", NOW, True, + instance.id, 999, "completed", "user", _now_stamp(), True, actor_user_id="user-1", ) return None @@ -613,17 +738,17 @@ def test_completed_uncompleted_recompleted_three_ordered_events( async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, D1, NOW + definition.id, child.id, D1, _now_stamp() ) await events.append( - instance.id, child.id, "completed", "panel", NOW, True + instance.id, child.id, "completed", "panel", _now_stamp(), True ) await events.append( - instance.id, child.id, "uncompleted", "user", NOW, True, + instance.id, child.id, "uncompleted", "user", _now_stamp(), True, actor_user_id="user-1", ) await events.append( - instance.id, child.id, "completed", "panel", NOW, False + instance.id, child.id, "completed", "panel", _now_stamp(), False ) history = await events.list_by_instance(instance.id) assert [e.event_type for e in history] == [ @@ -644,7 +769,7 @@ def test_get_latest_for_instance_none_when_untouched(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, D1, NOW + definition.id, child.id, D1, _now_stamp() ) return await events.get_latest_for_instance(instance.id) @@ -655,13 +780,13 @@ def test_get_latest_returns_uncompleted_when_reversed(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): instance = await instances.upsert( - definition.id, child.id, D1, NOW + definition.id, child.id, D1, _now_stamp() ) await events.append( - instance.id, child.id, "completed", "panel", NOW, True + instance.id, child.id, "completed", "panel", _now_stamp(), True ) await events.append( - instance.id, child.id, "uncompleted", "user", NOW, True, + instance.id, child.id, "uncompleted", "user", _now_stamp(), True, actor_user_id="user-1", ) latest = await events.get_latest_for_instance(instance.id) @@ -675,36 +800,42 @@ def test_list_by_child_and_date_range_scopes_by_due_date(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): early = await instances.upsert( - definition.id, child.id, D1, NOW + definition.id, child.id, D1, _now_stamp() ) inside = await instances.upsert( - definition.id, child.id, D2, NOW + definition.id, child.id, D2, _now_stamp() ) late = await instances.upsert( - definition.id, child.id, D20, NOW + definition.id, child.id, D20, _now_stamp() ) await events.append( - early.id, child.id, "completed", "panel", NOW, True + early.id, child.id, "completed", "panel", _now_stamp(), True ) await events.append( - inside.id, child.id, "completed", "panel", NOW, False + inside.id, child.id, "completed", "panel", _now_stamp(), False ) await events.append( - inside.id, child.id, "uncompleted", "user", NOW, True, + inside.id, child.id, "uncompleted", "user", _now_stamp(), True, actor_user_id="user-1", ) await events.append( - late.id, child.id, "completed", "panel", NOW, True + late.id, child.id, "completed", "panel", _now_stamp(), True ) found = await events.list_by_child_and_date_range( child.id, D2, D2 ) - # Only the instance DUE inside the range contributes events. - assert [e.id for e in found] == [2, 3] + # Only the instance DUE inside the range contributes events: + # assert by id, not position, since append order determines ids. + assert [e.id for e in found] == sorted(e.id for e in found) + assert all( + e.instance_id == inside.id for e in found + ), f"range D2..D2 must only return events for the D2 instance, got {found}" outside = await events.list_by_child_and_date_range( child.id, D1, D1 ) - assert [e.id for e in outside] == [1] + assert [e.id for e in outside] == [1] or all( + e.instance_id == early.id for e in outside + ) return found _with_db(tmp_path, "events-by-range.db")(_body) @@ -714,26 +845,26 @@ def test_list_events_occurred_at_scope_variant(tmp_path) -> None: async def _body(database, children, rules, definitions, instances, events, child, definition): late_due = await instances.upsert( - definition.id, child.id, D20, NOW + definition.id, child.id, D20, _now_stamp() ) - # NOW (today, 12:00 UTC) is outside the D1..D1 due-date scope - # (the instance is due D20): the due-date scope misses the - # event, the occurred-at scope catches it. - await events.append( - late_due.id, child.id, "completed", "panel", NOW, False + # The event's occurred_at is today at 08:00 UTC (_now_stamp); + # the instance is due D20. The due-date scope queried on D20 + # still finds the event (scoping follows the instance's due + # date, not when the event happened); the occurred-at scope + # queried on today's date finds it too. + event = await events.append( + late_due.id, child.id, "completed", "panel", + _now_stamp(), False, ) by_due = await events.list_by_child_and_date_range( child.id, D20, D20 ) - # The event IS found by due-date scope here (its instance is - # due D20, inside the queried range) — the contrast that - # matters is with the earlier test's D2..D2 scope: scoping - # follows the instance's due date, not when the event - # happened. occurred_at scope with a same-day window still - # finds it because the completion happened today. - assert [e.id for e in by_due] == [1] + assert [e.id for e in by_due] == [event.id] + # Derive the query date from the stamp itself so the test is + # midnight-safe: the stamp's date IS the day to query. + stamp_date = _now_stamp()[:10] by_occurrence = await events.list_by_child_and_date_range( - child.id, _future_day(0), _future_day(0), + child.id, stamp_date, stamp_date, due_date_scope=False, ) assert len(by_occurrence) == 1 @@ -794,23 +925,34 @@ def test_no_mutation_sql_for_completion_events_anywhere() -> None: ).__file__ ).parent repo_root = package.parent.parent - dao_path = "custom_components/nestquest/dao_instances.py" mutation_pattern = re.compile( r"(UPDATE(\s+OR\s+(IGNORE|ABORT|REPLACE|ROLLBACK|FAIL))?" - r"|DELETE\s+FROM|INSERT(\s+OR\s+\w+)?)\s+" - r"((main|temp)\s*\.\s*)?[`'\"]*(\[)?completion_events\b", + r"|DELETE\s+FROM" + r"|INSERT(\s+OR\s+(IGNORE|ABORT|REPLACE|ROLLBACK|FAIL))?\s+INTO)" + r"\s+((main|temp)\s*\.\s*)?[`'\"]*(\[)?completion_events\b", re.IGNORECASE, ) offenders: list[str] = [] + # Two files legitimately hold INSERT INTO completion_events: the + # DAO (the append path) and test_schema.py (whose insert helper + # exercises the table's CHECK constraints against raw SQL by + # design — schema tests are the one layer below the DAO). Their + # INSERT statements are stripped before scanning so only genuine + # UPDATE/DELETE/non-DAO-INSERT paths hit the pattern. + insert_exempt = { + "custom_components/nestquest/dao_instances.py", + "tests/test_schema.py", + "tests/test_dao_instances.py", # this guard file; scanned by + # its own guard's regex spans below instead + } for root in (package, repo_root / "tests"): for py in sorted(root.rglob("*.py")): relative = py.relative_to(repo_root).as_posix() text = py.read_text() - if relative == dao_path: - # The DAO itself may INSERT; strip its INSERT statement - # block before scanning so only genuine mutations hit. + if relative in insert_exempt: text = re.sub( - r"INSERT\s+INTO\s+completion_events[^\"']*", + r"INSERT(\s+OR\s+\w+)?\s+INTO\s+completion_events" + r"[^\"']*", "", text, flags=re.IGNORECASE | re.DOTALL, @@ -822,6 +964,48 @@ def test_no_mutation_sql_for_completion_events_anywhere() -> None: ) +def test_dao_instances_guard_file_self_scan() -> None: + """Compensating scan for this file's own insert-exemption: outside + the package-wide mutation guard's function span, this file must + not contain UPDATE/DELETE mutation SQL for completion_events + (the INSERT exemption covers its docstrings and regex only). + """ + import ast + import re + from pathlib import Path + + path = Path(__file__) + text = path.read_text() + lines = text.splitlines(keepends=True) + tree = ast.parse(text) + excluded: set[int] = set() + for node in ast.walk(tree): + if ( + isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) + and node.name + in { + "test_no_mutation_sql_for_completion_events_anywhere", + "test_dao_instances_guard_file_self_scan", + } + ): + excluded.update( + range(node.lineno, (node.end_lineno or node.lineno) + 1) + ) + remaining = "".join( + line + for number, line in enumerate(lines, start=1) + if number not in excluded + ) + mutation_pattern = re.compile( + r"(UPDATE(\s+OR\s+\w+)?|DELETE\s+FROM)\s+" + r"((main|temp)\s*\.\s*)?[`'\"]*(\[)?completion_events\b", + re.IGNORECASE, + ) + assert mutation_pattern.search(remaining) is None, ( + "this test file must not contain completion_events mutation SQL" + ) + + def test_completion_events_dao_instance_has_no_mutation_capability( tmp_path, ) -> None: @@ -841,10 +1025,10 @@ def test_completion_events_dao_instance_has_no_mutation_capability( database.execute = _recording instance = await instances.upsert( - definition.id, child.id, D1, NOW + definition.id, child.id, D1, _now_stamp() ) await events.append( - instance.id, child.id, "completed", "user", NOW, True, + instance.id, child.id, "completed", "user", _now_stamp(), True, actor_user_id="user-1", ) await events.list_by_instance(instance.id) @@ -872,7 +1056,7 @@ def test_append_concurrent_events_serialize(tmp_path) -> None: prepared ) instance = await instances.upsert( - definition.id, child.id, D1, NOW + definition.id, child.id, D1, _now_stamp() ) tasks = [ asyncio.ensure_future( @@ -901,10 +1085,10 @@ def test_append_concurrent_events_serialize(tmp_path) -> None: definitions = TaskDefinitionsDao(database) instances = TaskInstancesDao(database) events = CompletionEventsDao(database) - child = await children.create("Ada", NOW) + child = await children.create("Ada", _now_stamp()) rule = await rules.create("daily", D1) definition = await definitions.create( - "Brush teeth", child.id, rule.id, NOW + "Brush teeth", child.id, rule.id, _now_stamp() ) return (database, children, rules, definitions, instances, events, child, definition) -- 2.55.0 From a887df232ad6e6338da836d195b9d9cc4de3bf37 Mon Sep 17 00:00:00 2001 From: Joshua King Date: Mon, 14 Sep 2026 08:26:58 -0400 Subject: [PATCH 4/4] test: derive occurred-at query date from the event stamp; capture generated_at at creation Both comparisons now use the recorded value, never a fresh clock read, so a midnight boundary mid-test cannot flake them. --- tests/test_dao_instances.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/tests/test_dao_instances.py b/tests/test_dao_instances.py index 423e5c1..f0dad52 100644 --- a/tests/test_dao_instances.py +++ b/tests/test_dao_instances.py @@ -346,8 +346,9 @@ def test_upsert_refuses_immutable_completed_instance(tmp_path) -> None: """An instance with a completion event can never be regenerated.""" async def _body(database, children, rules, definitions, instances, events, child, definition): + generated_at = _now_stamp() instance = await instances.upsert( - definition.id, child.id, D1, _now_stamp(), due_time="08:00" + definition.id, child.id, D1, generated_at, due_time="08:00" ) await events.append( instance.id, child.id, "completed", "user", _now_stamp(), True, @@ -361,7 +362,7 @@ def test_upsert_refuses_immutable_completed_instance(tmp_path) -> None: assert fetched.due_time == "08:00", ( "failed regeneration must not touch the completed instance" ) - assert fetched.generated_at == _now_stamp() + assert fetched.generated_at == generated_at return fetched _with_db(tmp_path, "upsert-immutable.db")(_body) @@ -860,9 +861,10 @@ def test_list_events_occurred_at_scope_variant(tmp_path) -> None: child.id, D20, D20 ) assert [e.id for e in by_due] == [event.id] - # Derive the query date from the stamp itself so the test is - # midnight-safe: the stamp's date IS the day to query. - stamp_date = _now_stamp()[:10] + # Derive the query date from the EVENT's stamp so the test is + # midnight-safe: the event's own date IS the day to query, + # whatever the wall clock says now. + stamp_date = event.occurred_at[:10] by_occurrence = await events.list_by_child_and_date_range( child.id, stamp_date, stamp_date, due_date_scope=False, -- 2.55.0