Add actor_child_id to completion_events (D-008)

Per IMPLEMENTATION-ORDER.md 1.5: a nullable actor_child_id column records which panel profile was tapped; admin events stay NULL. - schema.py: completion_events table body extracted to COMPLETION_EVENTS_TABLE_SQL; the new column carries a FK to children(id) and the actor-pair coherence is extended to a second CHECK: 'user' events require actor_user_id and forbid actor_child_id, 'panel' events require actor_child_id and forbid actor_user_id. - migrations.py: migration 6 rebuilds legacy-shaped completion_events via the canonical procedure (CHECKs cannot be added to an existing table), backfilling each legacy panel row to its own child_id (pre-D-008 panel completions were always the tapped child) via INSERT..SELECT — no UPDATE or DELETE path exists, so the append-only guarantee is untouched; ids preserved; PRAGMA foreign_key_check verified in-transaction. - dao_instances.py: append() takes actor_child_id, validates both actor shapes (panel requires it, user forbids it) and the tapped child's existence under the connection lock; the record and column list grow accordingly. - tests: coherence CHECK matrix (5 bad shapes), unknown actor_child_id FK, panel/user round-trips at DDL and DAO layers, migration-6 backfill test on a frozen v2-shape seed; the completion_events leak guard gains justified INSERT exemptions for the migration layer (UPDATE/DELETE stay forbidden everywhere).
2026-09-15 10:25:50 -04:00
parent 25f1479444
commit fe90064b65
7 changed files with 318 additions and 66 deletions
@@ -106,6 +106,7 @@ class CompletionEventRecord:
    event_type: str
    actor_source: str
    actor_user_id: str | None
+    actor_child_id: int | None
    occurred_at: str
    was_on_time: bool | None

@@ -115,7 +116,7 @@ _INSTANCE_COLUMNS = (
 )
 _EVENT_COLUMNS = (
    "id, instance_id, child_id, event_type, actor_source, actor_user_id, "
-    "occurred_at, was_on_time"
+    "actor_child_id, occurred_at, was_on_time"
 )

 #: The only event types the append path accepts; the schema CHECK is
@@ -144,8 +145,9 @@ def _event_from_row(row: tuple) -> CompletionEventRecord:
        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]),
+        actor_child_id=row[6],
+        occurred_at=row[7],
+        was_on_time=None if row[8] is None else bool(row[8]),
    )


@@ -396,19 +398,21 @@ class CompletionEventsDao:
        was_on_time: bool | None,
        *,
        actor_user_id: str | None = None,
+        actor_child_id: int | 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.
-        ``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.
+        ``actor_source`` is 'user' (an admin action: ``actor_user_id``
+        set, ``actor_child_id`` NULL) or 'panel' (a panel tap: no user
+        id, and ``actor_child_id`` carrying the tapped child profile,
+        D-008); the actor-pair CHECKs enforce this shape at the schema
+        level.  ``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, child and — for panel events — the tapped child must
+        exist — validated atomically under the connection lock.
        """
        if event_type not in (EVENT_COMPLETED, EVENT_UNCOMPLETED):
            raise ValueError(
@@ -428,6 +432,15 @@ class CompletionEventsDao:
            raise ValueError(
                "actor_source 'panel' must not carry actor_user_id"
            )
+        if actor_source == "panel" and actor_child_id is None:
+            raise ValueError(
+                "actor_source 'panel' requires actor_child_id "
+                "(the tapped child profile)"
+            )
+        if actor_source == "user" and actor_child_id is not None:
+            raise ValueError(
+                "actor_source 'user' must not carry actor_child_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 "
@@ -458,16 +471,28 @@ class CompletionEventsDao:
                )
                if child is None:
                    raise ValueError(f"child {child_id} does not exist")
+                if actor_child_id is not None:
+                    tapped = await self._database.fetch_one(
+                        "SELECT 1 FROM children WHERE id = ?",
+                        (actor_child_id,),
+                    )
+                    if tapped is None:
+                        raise ValueError(
+                            f"actor_child_id {actor_child_id} does not "
+                            "reference an existing child"
+                        )
                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 (?, ?, ?, ?, ?, ?, ?)",
+                    "event_type, actor_source, actor_user_id, "
+                    "actor_child_id, occurred_at, was_on_time) "
+                    "VALUES (?, ?, ?, ?, ?, ?, ?, ?)",
                    (
                        instance_id,
                        child_id,
                        event_type,
                        actor_source,
                        actor_user_id,
+                        actor_child_id,
                        occurred_at,
                        None if was_on_time is None else int(was_on_time),
                    ),
@@ -25,11 +25,12 @@ pre-D-008 databases to the multi-assignee model: it creates
 ``quest_definition_assignees`` and, when ``quest_definitions`` still
 carries a single ``child_id`` column, rebuilds the table without it,
 copying each definition's assignee across.  Migration 4 adds the
-windows table and migration 5 rebuilds ``quest_instances`` onto the
-widened (definition_id, child_id, due_date, window) key.  Each callable
-migration runs inside its own transaction together with its version
-stamp, so a crash mid-step rolls the statements and the stamp back
-together.
+windows table, migration 5 rebuilds ``quest_instances`` onto the
+widened (definition_id, child_id, due_date, window) key, and migration
+6 rebuilds ``completion_events`` with the D-008 ``actor_child_id``
+column.  Each callable migration runs inside its own transaction
+together with its version stamp, so a crash mid-step rolls the
+statements and the stamp back together.

 Migration entries are either an ordered sequence of SQL statements or
 an async callable taking ``(database, target_version)``.  Callables
@@ -73,6 +74,7 @@ from typing import Union

 from .db import NestQuestDatabase
 from .schema import (
+    COMPLETION_EVENTS_TABLE_SQL,
    QUEST_DEFINITIONS_TABLE_SQL,
    QUEST_INSTANCES_TABLE_SQL,
    SCHEMA_V1_STATEMENTS,
@@ -326,6 +328,74 @@ async def _widen_instance_key(
        await database.execute("PRAGMA foreign_keys = ON")


+async def _add_actor_child_id(
+    database: NestQuestDatabase, target_version: int
+) -> None:
+    """Bring ``completion_events`` to the D-008 actor-child shape.
+
+    The ``actor_child_id`` column records which panel profile was
+    tapped; it cannot be added with ``ALTER TABLE ADD COLUMN`` alone
+    because the canonical shape also carries an actor-pair coherence
+    CHECK (panel events require it, user events forbid it), and CHECK
+    constraints cannot be added to an existing table.  Databases with
+    the legacy shape are therefore rebuilt via SQLite's canonical
+    procedure, backfilling each legacy panel row's ``actor_child_id``
+    to ``child_id`` — pre-D-008 panel completions were always recorded
+    against the tapped child themselves — while admin rows stay NULL.
+    Row ids are preserved so ``quest_instances`` references and the
+    append-only history stay intact.  Fresh databases already match
+    the canonical shape and only stamp.
+
+    The rebuild runs with ``foreign_keys = OFF`` and verifies
+    ``PRAGMA foreign_key_check`` inside the transaction before the
+    stamp.  The copy is an INSERT..SELECT, not an UPDATE: the
+    completion_events table keeps its append-only guarantee.
+    """
+    legacy_shape = not await _table_has_column(
+        database, "completion_events", "actor_child_id"
+    )
+    await database.execute("PRAGMA foreign_keys = OFF")
+    try:
+        async with database.transaction():
+            if legacy_shape:
+                await database.execute(
+                    "CREATE TABLE completion_events_rebuilt "
+                    f"{COMPLETION_EVENTS_TABLE_SQL}"
+                )
+                await database.execute(
+                    "INSERT INTO completion_events_rebuilt "
+                    "(id, instance_id, child_id, event_type, actor_source, "
+                    "actor_user_id, actor_child_id, occurred_at, "
+                    "was_on_time) "
+                    "SELECT id, instance_id, child_id, event_type, "
+                    "actor_source, actor_user_id, "
+                    "CASE WHEN actor_source = 'panel' THEN child_id "
+                    "ELSE NULL END, "
+                    "occurred_at, was_on_time "
+                    "FROM completion_events ORDER BY id"
+                )
+                await database.execute("DROP TABLE completion_events")
+                await database.execute(
+                    "ALTER TABLE completion_events_rebuilt "
+                    "RENAME TO completion_events"
+                )
+                violations = await database.fetch_all(
+                    "PRAGMA foreign_key_check"
+                )
+                if violations:
+                    raise RuntimeError(
+                        "completion_events rebuild produced foreign-key "
+                        f"violations: {violations!r}"
+                    )
+                LOGGER.info(
+                    "Added actor_child_id to completion_events; legacy "
+                    "panel rows backfilled to their own child_id"
+                )
+            await stamp_schema_version(database, target_version)
+    finally:
+        await database.execute("PRAGMA foreign_keys = ON")
+
+
 #: The ordered migration list.  Append-only: never edit an applied
 #: entry, add the next one instead.  (Migration 1's content was
 #: rewritten pre-release per D-007 — no version 1 database shipped.)
@@ -340,6 +410,10 @@ MIGRATIONS: Sequence[MigrationStep] = [
    # Migration 5 (D-008): rebuild quest_instances onto the widened
    # (definition_id, child_id, due_date, window) key.
    _widen_instance_key,
+    # Migration 6 (D-008): rebuild completion_events with the
+    # actor_child_id column and its actor-pair coherence CHECK,
+    # backfilling legacy panel rows to their own child_id.
+    _add_actor_child_id,
 ]


@@ -159,11 +159,12 @@ self-describing without joins.
 Actor policy: ``actor_source`` distinguishes an authenticated HA user
 (``'user'``, with ``actor_user_id`` holding the HA user id) from a
 panel tap with no user context (``'panel'``, with ``actor_user_id``
-NULL — the tapped child profile is already ``child_id``).  A coherence
-CHECK makes the pair shape non-negotiable: ``'user'`` requires
-``actor_user_id``, ``'panel'`` forbids it.  Other actor sources are
-rejected; if an unattended system actor is ever needed, that is a
-schema migration, not a silent widening.
+NULL — the tapped child profile is ``actor_child_id``, D-008).  Two
+coherence CHECKs make both shapes non-negotiable: ``'user'`` requires
+``actor_user_id`` and forbids ``actor_child_id``; ``'panel'`` requires
+``actor_child_id`` (the tapped profile) and forbids ``actor_user_id``.
+Other actor sources are rejected; if an unattended system actor is
+ever needed, that is a schema migration, not a silent widening.

 On-time policy: ``was_on_time`` is required (integer 0 or 1) for
 ``completed`` events — at completion time the due date is known and the
@@ -344,29 +345,42 @@ SCHEMA_V1_QUEST_INSTANCES_DDL: list[str] = [
    """,
 ]

-SCHEMA_V1_COMPLETION_EVENTS_DDL: list[str] = [
-    """
-    CREATE TABLE IF NOT EXISTS completion_events (
-        id INTEGER PRIMARY KEY AUTOINCREMENT,
-        instance_id INTEGER NOT NULL REFERENCES quest_instances(id),
-        child_id INTEGER NOT NULL REFERENCES children(id),
-        event_type TEXT NOT NULL CHECK (event_type IN ('completed', 'uncompleted')),
-        actor_source TEXT NOT NULL CHECK (actor_source IN ('user', 'panel')),
-        actor_user_id TEXT,
-        occurred_at TEXT NOT NULL,
-        was_on_time INTEGER CHECK (
-            was_on_time IS NULL
-            OR (typeof(was_on_time) = 'integer' AND was_on_time IN (0, 1))
-        ),
-        CHECK (
-            (actor_source = 'user' AND actor_user_id IS NOT NULL)
-            OR (actor_source = 'panel' AND actor_user_id IS NULL)
-        ),
-        CHECK (
-            (event_type = 'completed' AND was_on_time IS NOT NULL)
-            OR (event_type = 'uncompleted')
-        )
+#: Column body of ``completion_events``, shared between the v1 DDL
+#: below and the actor-child rebuild migration, which must recreate the
+#: table in exactly this shape (D-008: ``actor_child_id`` records which
+#: panel profile was tapped — set for panel events, NULL for admin
+#: ones; legacy panel rows backfill to ``child_id``, the child they
+#: always were).
+COMPLETION_EVENTS_TABLE_SQL = """(
+    id INTEGER PRIMARY KEY AUTOINCREMENT,
+    instance_id INTEGER NOT NULL REFERENCES quest_instances(id),
+    child_id INTEGER NOT NULL REFERENCES children(id),
+    event_type TEXT NOT NULL CHECK (event_type IN ('completed', 'uncompleted')),
+    actor_source TEXT NOT NULL CHECK (actor_source IN ('user', 'panel')),
+    actor_user_id TEXT,
+    actor_child_id INTEGER REFERENCES children(id),
+    occurred_at TEXT NOT NULL,
+    was_on_time INTEGER CHECK (
+        was_on_time IS NULL
+        OR (typeof(was_on_time) = 'integer' AND was_on_time IN (0, 1))
+    ),
+    CHECK (
+        (actor_source = 'user' AND actor_user_id IS NOT NULL)
+        OR (actor_source = 'panel' AND actor_user_id IS NULL)
+    ),
+    CHECK (
+        (actor_source = 'user' AND actor_child_id IS NULL)
+        OR (actor_source = 'panel' AND actor_child_id IS NOT NULL)
+    ),
+    CHECK (
+        (event_type = 'completed' AND was_on_time IS NOT NULL)
+        OR (event_type = 'uncompleted')
    )
+)"""
+
+SCHEMA_V1_COMPLETION_EVENTS_DDL: list[str] = [
+    f"""
+    CREATE TABLE IF NOT EXISTS completion_events {COMPLETION_EVENTS_TABLE_SQL}
    """,
 ]