diff --git a/alembic/versions/0092_note_verification.py b/alembic/versions/0092_note_verification.py new file mode 100644 index 0000000..52f73d1 --- /dev/null +++ b/alembic/versions/0092_note_verification.py @@ -0,0 +1,80 @@ +"""a note can carry its own check — verify_with, expires_when, verified_at +(milestone 317 step 1) + +Revision ID: 0092 +Revises: 0091 +Create Date: 2026-08-28 + +The sibling of 0090, which gave rules the same three columns. Same +distinction, one table over: + +A NORM is a decision — no truth value, and it changes only when its author +changes it, which they know they did. A CONSTRAINT is a fact about someone +else's software, and nobody is present when it goes false. + +Notes hold far more constraints than rules do, and hold them for longer. A +cross-project reference note asserting what a signing service does on a +duplicate upload, or how a forge numbers its CI runs, is believed by every +project that reads it, and there is nothing in the record that says when +anyone last looked. `note_supersessions` only fires once a human has read +the note, disagreed, and written the correction — which is the case where +the note was already believed. + +Three nullable columns: + +- `verify_with` — how to tell whether this is still true. A command, a path, + a URL, a query. Prose is allowed; something runnable is better. +- `expires_when` — the STATE under which it stops being true. Deliberately + not a date: constraints do not expire on a schedule, they expire when the + world underneath them moves. +- `verified_at` — when the check last passed. NULL means never checked, and + sorts FIRST in the sweep: unexamined outranks examined-long-ago. + +WHICH ROWS THESE ARE FOR. `notes` is one table holding notes, tasks, +snippets and processes, so these columns land on all of them. Only non-task, +non-snippet records are OFFERED them (milestone 317 decisions 1 and 2, gated +at the service in step 2): a task's decay is its status, and a snippet +already carries a richer, location-aware verdict in `data.verification`. The +columns exist on the other rows and stay null there; a gate that lives in +the schema would have meant a partial index or a CHECK across three columns +to express something the write path can say in two lines. + +All three optional, because most notes should set none of them — the whole +value of the sweep is that its output is short. A null `verify_with` is not +an omission; it is the honest marker of "this one is a decision, and there +is nothing to go and check." + +No CHECK constraint is involved, so rule 36 does not apply. Nothing is +backfilled: a migration cannot invent a check. +""" +import sqlalchemy as sa +from alembic import op + +revision = "0092" +down_revision = "0091" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column("notes", sa.Column("verify_with", sa.Text(), nullable=True)) + op.add_column("notes", sa.Column("expires_when", sa.Text(), nullable=True)) + op.add_column( + "notes", + sa.Column("verified_at", sa.DateTime(timezone=True), nullable=True), + ) + # No index, for 0090's reason — the sweep runs when a human asks, never on + # a request path — but the margin is thinner here and worth naming. `rules` + # is hundreds of rows; `notes` is thousands and grows with every session. + # + # Still a sequential scan's job at this size, and an index on + # (verified_at) filtered to `verify_with IS NOT NULL` would be maintained + # on every note write to serve one operator-initiated query. If step 3's + # live acceptance measures otherwise, add it there against a real plan + # rather than guessing here. + + +def downgrade() -> None: + op.drop_column("notes", "verified_at") + op.drop_column("notes", "expires_when") + op.drop_column("notes", "verify_with") diff --git a/src/scribe/models/note.py b/src/scribe/models/note.py index bef409c..f9d51b3 100644 --- a/src/scribe/models/note.py +++ b/src/scribe/models/note.py @@ -100,6 +100,34 @@ class Note(Base, TimestampMixin, SoftDeleteMixin): # snippets.backfill_snippet_data filled them at startup; readers still fall # back to parsing the body when it is absent (snippet_fields). data: Mapped[dict | None] = mapped_column(JSONB, nullable=True) + # The three fields that tell a CONSTRAINT apart from a NORM (milestone + # 317, migration 0092) — the same trio `rules` carries, and for the same + # reason. A norm is a decision: no truth value, changes only when its + # author changes it. A constraint asserts a fact about someone else's + # software and goes false with nobody watching. Only constraints get a + # check. + # + # `verify_with` is how to check it is still true; `expires_when` is the + # STATE that ends it, deliberately not a date — constraints expire when + # the ground moves, not on a schedule. `verified_at` NULL means never + # checked and sorts FIRST in the sweep: unexamined outranks + # examined-long-ago. + # + # These sit on `notes`, so every kind of row in this table has them, but + # only non-task, non-snippet records are OFFERED them (gated in + # services/notes.py). A task's decay is its status — a done issue records + # what happened and cannot go false — and a snippet already carries a + # richer, location-aware verdict in `data.verification`. On those rows + # these stay null, which is also what they mean. + # + # Most notes should leave all three empty. A null `verify_with` is not a + # gap; it is the marker for "this is a decision, there is nothing to go + # and check", and the sweep is only worth reading while that holds. + verify_with: Mapped[str | None] = mapped_column(Text, nullable=True) + expires_when: Mapped[str | None] = mapped_column(Text, nullable=True) + verified_at: Mapped[datetime | None] = mapped_column( + DateTime(timezone=True), nullable=True + ) __table_args__ = ( Index("ix_notes_tags", "tags", postgresql_using="gin"), @@ -140,6 +168,14 @@ class Note(Base, TimestampMixin, SoftDeleteMixin): "is_task": self.is_task, "note_type": self.note_type or "note", "task_kind": self.task_kind, + # Serialized unconditionally, like every other field a given row + # kind may not use (recurrence, started_at, the task fields). The + # DERIVED "last_verified" label is the one that appears only when + # a check exists — a raw projection of the row should not make a + # client branch on which keys are present. + "verify_with": self.verify_with or "", + "expires_when": self.expires_when or "", + "verified_at": iso(self.verified_at), "created_at": iso(self.created_at), "updated_at": iso(self.updated_at), } diff --git a/src/scribe/services/backup.py b/src/scribe/services/backup.py index dd23826..552a8a9 100644 --- a/src/scribe/services/backup.py +++ b/src/scribe/services/backup.py @@ -50,8 +50,12 @@ logger = logging.getLogger(__name__) # ones (reference/hook) travel too, cheaply, and the next refresh refreshes them. # v10 (2026-08) added projects.inception + project_rulebook_exclusions # (milestone 297): the WHY a project inherits what it does, and its opt-outs. +# v11 (2026-08) added the note verification trio — notes.verify_with / +# expires_when / verified_at (milestone 317). NOT an audit of the notes +# section: it carries 16 of the 26 `notes` columns, and #3182 tracks the nine that +# have been missing since long before this. # Bump when the serialized schema changes. -BACKUP_VERSION = 10 +BACKUP_VERSION = 11 # Every table this backup carries, by its REAL name. Paired with _NOT_INCLUDED # below, these two lists must together account for the entire schema — which is @@ -291,6 +295,19 @@ def _milestone_rows(rows) -> list[dict]: def _note_rows(rows) -> list[dict]: + # INCOMPLETE, and knowingly so — see #3182. This carries 16 of the + # `notes` table's 26 columns. `note_type`, `task_kind`, `arose_from_id`, `data`, + # `description`, `recurrence_rule`, `recurrence_next_spawn_at`, + # `started_at` and `completed_at` are all absent, so a restore flattens + # every snippet and process into a plain note and every issue and spike + # into `work`. That predates the verification trio below and is tracked + # separately rather than fixed in passing: `arose_from_id` points at + # another note and needs the same second pass `parent_id` gets, which is + # a change with its own trap and deserves its own tests. + # + # The table-coverage guard cannot see this. It asserts that every TABLE in + # Base.metadata is either backed up or declared not-included; nothing + # checks columns, which is exactly how nine of them went missing quietly. return [ { "id": n.id, "user_id": n.user_id, "title": n.title, "body": n.body, @@ -300,6 +317,12 @@ def _note_rows(rows) -> list[dict]: "due_date": n.due_date.isoformat() if n.due_date else None, "created_at": n.created_at.isoformat(), "updated_at": n.updated_at.isoformat(), + # The verification trio (milestone 317, migration 0092). These + # travel because they are operator judgment — "somebody checked + # this fact, and this is when" — which nothing can recompute. + "verify_with": n.verify_with, + "expires_when": n.expires_when, + "verified_at": n.verified_at.isoformat() if n.verified_at else None, } for n in rows ] @@ -752,6 +775,13 @@ async def _restore_v1(data: dict) -> dict: due_date=_d(n_data.get("due_date")), created_at=_dt(n_data.get("created_at")), updated_at=_dt(n_data.get("updated_at")), + verify_with=n_data.get("verify_with"), + expires_when=n_data.get("expires_when"), + # _dt_or_none, NOT _dt: an absent stamp must stay absent. _dt + # substitutes now(), which would restore every never-checked + # note as checked at the moment of the restore — inverting the + # one signal the sweep reads. + verified_at=_dt_or_none(n_data.get("verified_at")), ) session.add(note) await session.flush() @@ -888,6 +918,10 @@ async def _restore_v2(data: dict) -> dict: due_date=_d(n_data.get("due_date")), created_at=_dt(n_data.get("created_at")), updated_at=_dt(n_data.get("updated_at")), + verify_with=n_data.get("verify_with"), + expires_when=n_data.get("expires_when"), + # _dt_or_none — see the note on the other restore path. + verified_at=_dt_or_none(n_data.get("verified_at")), ) session.add(note) await session.flush() diff --git a/tests/test_services_backup.py b/tests/test_services_backup.py index a5d78bc..f6603dd 100644 --- a/tests/test_services_backup.py +++ b/tests/test_services_backup.py @@ -16,12 +16,87 @@ import pytest from scribe.services import backup -def test_backup_version_is_v8(): - """v7 added code_shapes (#2787), v8 its history (#2793). The bump is the - point of the test — a payload section added without moving the version - produces backups that are structurally different and indistinguishable - by inspection.""" - assert backup.BACKUP_VERSION == 10 +def test_backup_version_is_current(): + """The bump is the point of the test — a payload section added without + moving the version produces backups that are structurally different and + indistinguishable by inspection. + + (Named for the number it asserted until v10, which is exactly the drift a + name-carrying-a-value invites; it now says what it checks.)""" + assert backup.BACKUP_VERSION == 11 + + +def _exportable_note(**over): + """A Note-shaped stand-in for the pure row helper. SimpleNamespace, not a + MagicMock: `_note_rows` calls .isoformat() on the timestamps, and a mock + would happily return another mock instead of failing.""" + base = dict( + id=1, user_id=7, title="t", body="b", tags=["x"], parent_id=None, + project_id=None, milestone_id=None, status=None, priority=None, + due_date=None, created_at=datetime(2026, 1, 1, tzinfo=timezone.utc), + updated_at=datetime(2026, 1, 2, tzinfo=timezone.utc), + verify_with=None, expires_when=None, verified_at=None, + ) + base.update(over) + return SimpleNamespace(**base) + + +def test_note_rows_carry_the_verification_trio(): + """Operator judgment — somebody checked this fact, and this is when — + which nothing downstream can recompute (milestone 317).""" + [row] = backup._note_rows([_exportable_note( + verify_with="curl the AMO docs", + expires_when="AMO starts allowing re-signing", + verified_at=datetime(2026, 8, 28, tzinfo=timezone.utc), + )]) + assert row["verify_with"] == "curl the AMO docs" + assert row["expires_when"] == "AMO starts allowing re-signing" + assert row["verified_at"] == "2026-08-28T00:00:00+00:00" + + +def test_a_never_checked_note_exports_a_null_stamp_and_restores_as_one(): + """The round trip that matters. NULL `verified_at` means nobody has ever + looked, and it is what sorts FIRST in the sweep. Restoring it as now() — + which is what `_dt` would do — silently converts the sweep's top result + into its bottom one.""" + [row] = backup._note_rows([_exportable_note(verify_with="check the runner shell")]) + assert row["verified_at"] is None + assert backup._dt_or_none(row["verified_at"]) is None + # ...and the helper that must NOT be used here, for contrast. + assert backup._dt(row["verified_at"]) is not None + + +def test_the_note_section_gap_is_pinned_rather_than_silent(): + """#3182. `_note_rows` carries 16 of the `notes` table's 26 columns, and the + absences are not harmless: without `note_type` every snippet and process + restores as a plain note, and without `task_kind` every issue and spike + restores as `work`. + + The table-coverage guard cannot see this — it asserts that every TABLE is + backed up or declared excluded, and nothing checks COLUMNS, which is how + these went missing quietly. + + This test exists to make the gap loud and to make fixing it visible: when + #3182 lands, this list shrinks, and a reviewer sees exactly which fields + started travelling. It is not an endorsement of the omissions. + """ + from scribe.models.note import Note + + carried = set(backup._note_rows([_exportable_note()])[0]) + missing = {c.name for c in Note.__table__.columns} - carried + + assert missing == { + # Deliberate: trashed rows are not exported. + "deleted_at", + # NOT deliberate — the #3182 gap, in the order they hurt. + "note_type", # snippets and processes flatten into notes + "task_kind", # issues and spikes flatten into work + "arose_from_id", # every issue -> origin edge is dropped + "recurrence_rule", "recurrence_next_spawn_at", # recurring tasks stop + "started_at", "completed_at", # lifecycle history + "description", + "data", # self-heals: backfill_snippet_data rebuilds it + } def test_not_included_lists_the_known_gaps():