Two milestones: a note can carry its own check (317), and a rule keeps what it used to say (323) #135

Merged
bvandeusen merged 23 commits from dev into main 2026-08-31 00:01:15 -04:00
4 changed files with 232 additions and 7 deletions
Showing only changes of commit 2065781302 - Show all commits
@@ -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")
+36
View File
@@ -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),
}
+35 -1
View File
@@ -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()
+81 -6
View File
@@ -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():