From e08e999406bb62e690cb6c0d08d3fb32bd7c3bdd Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 27 Aug 2026 07:44:13 -0400 Subject: [PATCH] =?UTF-8?q?feat(rules):=20a=20rule=20can=20carry=20its=20o?= =?UTF-8?q?wn=20check=20=E2=80=94=20verify=5Fwith,=20expires=5Fwhen,=20ver?= =?UTF-8?q?ified=5Fat=20(#3095,=20milestone=20312=20step=201)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A rulebook holds two kinds of row in one table. A NORM is a decision: no truth value, changes only when its author changes it, and they know they did. A CONSTRAINT asserts a fact about someone else's software, and goes false with nobody present. Milestone 307's audit found nine stale sites; every one was a constraint, and not one norm had rotted. Three nullable columns so a rule can say how to check itself. expires_when is a STATE, 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 to come: unexamined outranks examined-long-ago. Most rules set none of the three; a null verify_with is the marker for "this is a decision, there is nothing to go and check," and it only reads that way while it stays honest. Nothing is backfilled and nothing is indexed. A migration cannot invent a check any more than 0088 could invent a trigger, and the sweep reads a whole rulebook — hundreds of rows, on operator demand, never on a request path. Also, in the backup service the fields had to pass through: - Restore now remaps arose_from_id through note_id_map. It has been exported since 0088 and silently dropped on the way back in ever since, so every restore lost every rule's provenance link. - _dt_or_none, because _dt substitutes now() for an absent value. That is right for created_at/updated_at and wrong here: a rule nobody ever checked would restore looking freshly checked and fall to the bottom of the sweep it should top. Column additions do not move BACKUP_VERSION; only new sections do, as when 0088 added when_to_apply/tier/arose_from_id to the same helper. Co-Authored-By: Claude Opus 5 (1M context) --- alembic/versions/0090_rule_verification.py | 64 ++++++++++++++++++++ src/scribe/models/rulebook.py | 23 ++++++++ src/scribe/services/backup.py | 27 +++++++++ tests/test_services_backup.py | 68 ++++++++++++++++++++-- 4 files changed, 177 insertions(+), 5 deletions(-) create mode 100644 alembic/versions/0090_rule_verification.py diff --git a/alembic/versions/0090_rule_verification.py b/alembic/versions/0090_rule_verification.py new file mode 100644 index 0000000..0ba64a9 --- /dev/null +++ b/alembic/versions/0090_rule_verification.py @@ -0,0 +1,64 @@ +"""a rule can carry its own check — verify_with, expires_when, verified_at +(milestone 312 step 1) + +Revision ID: 0090 +Revises: 0089 +Create Date: 2026-08-27 + +A rulebook holds two kinds of row in one table. A NORM is a decision: it has +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: a +runner's shell, a bot's config, a tool that exists. Nobody is present when +that goes false. + +Milestone 307's rulebook audit found nine stale sites. Every one was a +constraint; not one norm had rotted. One of them had been telling every +session to skip database-backed tests for weeks while the integration lane +sat green in the workflow. + +Three nullable columns, so a rule can say how to check itself: + +- `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. + +All three nullable and all three optional, because most rules should set +none of them. 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." +That signal only works if the field stays empty wherever it belongs empty. + +No CHECK constraint is involved, so rule 36 does not apply here. Nothing is +backfilled: a migration cannot invent a check any more than 0088 could +invent a trigger. +""" +import sqlalchemy as sa +from alembic import op + +revision = "0090" +down_revision = "0089" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column("rules", sa.Column("verify_with", sa.Text(), nullable=True)) + op.add_column("rules", sa.Column("expires_when", sa.Text(), nullable=True)) + op.add_column( + "rules", + sa.Column("verified_at", sa.DateTime(timezone=True), nullable=True), + ) + # No index on (verify_with, verified_at). The sweep this exists for reads + # an operator's whole rulebook — hundreds of rows, not millions — and runs + # when a human asks for it, never on a request path. An index here would + # be maintained on every rule write to serve a query that a sequential + # scan answers instantly. + + +def downgrade() -> None: + op.drop_column("rules", "verified_at") + op.drop_column("rules", "expires_when") + op.drop_column("rules", "verify_with") diff --git a/src/scribe/models/rulebook.py b/src/scribe/models/rulebook.py index d05da94..ac581cb 100644 --- a/src/scribe/models/rulebook.py +++ b/src/scribe/models/rulebook.py @@ -107,6 +107,26 @@ class Rule(Base, TimestampMixin, SoftDeleteMixin): tier: Mapped[str] = mapped_column(Text, default="always_on", server_default="always_on") why: Mapped[str | None] = mapped_column(Text, nullable=True) how_to_apply: Mapped[str | None] = mapped_column(Text, nullable=True) + # The three fields that tell a CONSTRAINT apart from a NORM (milestone + # 312). 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: every stale rule the + # 307 audit found was one, and no norm had rotted. + # + # `verify_with` is how to check the rule 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. + # + # Most rules 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 signal is only worth reading while that stays true. + 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 + ) # The record that caused this rule — the edge notes and tasks already # have. Rule 46's `why` names note 2813 in prose; this is that link as a # field, so it survives a rewording of the paragraph. @@ -126,6 +146,9 @@ class Rule(Base, TimestampMixin, SoftDeleteMixin): "tier": self.tier, "why": self.why or "", "how_to_apply": self.how_to_apply or "", + "verify_with": self.verify_with or "", + "expires_when": self.expires_when or "", + "verified_at": iso(self.verified_at), "arose_from_id": self.arose_from_id, "order_index": self.order_index, "created_at": iso(self.created_at), diff --git a/src/scribe/services/backup.py b/src/scribe/services/backup.py index 7140da1..dd23826 100644 --- a/src/scribe/services/backup.py +++ b/src/scribe/services/backup.py @@ -112,6 +112,18 @@ def _dt(val: str | None) -> datetime: return datetime.fromisoformat(val) if val else datetime.now(timezone.utc) +def _dt_or_none(val: str | None) -> datetime | None: + """Like _dt, but keeps an absent timestamp absent. + + _dt substitutes now() because created_at/updated_at must not be null. + For a nullable column that MEANS something by being empty, that default + is a lie: a rule nobody ever verified would restore looking verified at + the moment of the restore, and drop straight to the bottom of the sweep + it should have topped. + """ + return datetime.fromisoformat(val) if val else None + + def _d(val: str | None) -> date | None: return date.fromisoformat(val) if val else None @@ -385,6 +397,8 @@ def _rule_rows(rows) -> list[dict]: "title": r.title, "statement": r.statement, "why": r.why, "how_to_apply": r.how_to_apply, "order_index": r.order_index, "when_to_apply": r.when_to_apply, "tier": r.tier, + "verify_with": r.verify_with, "expires_when": r.expires_when, + "verified_at": r.verified_at.isoformat() if r.verified_at else None, "arose_from_id": r.arose_from_id, "created_at": r.created_at.isoformat(), "updated_at": r.updated_at.isoformat(), @@ -1007,6 +1021,19 @@ async def _restore_v2(data: dict) -> dict: # is the pre-0088 behaviour, so an old backup restores rules # that bind exactly as they did when it was taken. tier=r_data.get("tier") or "always_on", + verify_with=r_data.get("verify_with") or None, + expires_when=r_data.get("expires_when") or None, + # Restored as-is, NOT reset to null. `verified_at` records + # when someone last ran the check; a restore does not make + # that untrue, and clearing it would put every constraint at + # the top of the sweep with nothing having actually changed. + verified_at=_dt_or_none(r_data.get("verified_at")), + # Remapped through note_id_map like every other note edge. + # Exported since 0088 but dropped on the way back in until + # milestone 312 — a restore silently lost every rule's + # provenance link. SET NULL semantics apply here too: a + # source note that didn't restore leaves the rule intact. + arose_from_id=note_id_map.get(r_data.get("arose_from_id") or 0), order_index=r_data.get("order_index", 0), created_at=_dt(r_data.get("created_at")), updated_at=_dt(r_data.get("updated_at")), diff --git a/tests/test_services_backup.py b/tests/test_services_backup.py index 3eaac1c..a5d78bc 100644 --- a/tests/test_services_backup.py +++ b/tests/test_services_backup.py @@ -1,10 +1,13 @@ -"""Unit tests for the v4 backup export contract. +"""Unit tests for the backup export contract. -CI runs pytest with no database, so these cover the parts that don't need one: -the version/coverage constants, the pure join-table row helpers, and the export -dict shape (via a mocked session). Full FK-remapping round-trip is exercised -manually against a real DB (export a backup, confirm rulebooks appear). +This is the no-database lane, so these cover the parts that need none: the +version/coverage constants, the pure row helpers, and the export dict shape +(via a mocked session). The full FK-remapping round-trip needs real Postgres +and belongs in a `@pytest.mark.integration` module — it is not written yet, +which is why every row helper here is a plain function that can be tested +without a session. """ +from datetime import datetime, timezone from types import SimpleNamespace from unittest.mock import patch @@ -132,3 +135,58 @@ def test_supersession_rows_serialise_the_pair(): {"superseder_id": 9, "superseded_id": 4}, {"superseder_id": 9, "superseded_id": 5}, ] + + +def test_rule_rows_carry_the_verification_fields(): + """A rule's check must survive a backup. + + `verify_with`/`expires_when`/`verified_at` (milestone 312) say whether a + rule is a fact that can go false and when it was last confirmed. A backup + that drops them restores a rulebook that has forgotten which of its rules + can rot — the exact blindness the fields were added to end. + + Column additions do not bump BACKUP_VERSION; only new SECTIONS do. Same + call made for when_to_apply/tier/arose_from_id in 0088 (commit 6ddb8bf). + """ + checked = datetime(2026, 8, 27, 12, 0, tzinfo=timezone.utc) + row = SimpleNamespace( + id=1, topic_id=2, project_id=None, title="t", statement="s", + why="w", how_to_apply="h", order_index=0, + when_to_apply="when", tier="conditional", + verify_with="cat some/file", expires_when="the file grows a shell", + verified_at=checked, arose_from_id=99, + created_at=checked, updated_at=checked, + ) + out = backup._rule_rows([row])[0] + + assert out["verify_with"] == "cat some/file" + assert out["expires_when"] == "the file grows a shell" + assert out["verified_at"] == checked.isoformat() + # Provenance was exported from 0088 onward but silently dropped on the way + # back IN until milestone 312. Export side asserted here; the restore side + # remaps it through note_id_map. + assert out["arose_from_id"] == 99 + + +def test_rule_rows_keep_an_unverified_rule_unverified(): + """NULL verified_at means never checked, and it must round-trip as null. + + _dt substitutes now() so created_at/updated_at are never null. Reusing it + here would restore a rule nobody ever checked as though it had just been + checked — dropping it to the BOTTOM of the sweep it should top. That is + why _dt_or_none exists. + """ + row = SimpleNamespace( + id=1, topic_id=2, project_id=None, title="t", statement="s", + why=None, how_to_apply=None, order_index=0, + when_to_apply=None, tier="always_on", + verify_with=None, expires_when=None, verified_at=None, + arose_from_id=None, + created_at=datetime(2026, 8, 27, tzinfo=timezone.utc), + updated_at=datetime(2026, 8, 27, tzinfo=timezone.utc), + ) + assert backup._rule_rows([row])[0]["verified_at"] is None + assert backup._dt_or_none(None) is None + assert backup._dt_or_none("2026-08-27T12:00:00+00:00") == datetime( + 2026, 8, 27, 12, 0, tzinfo=timezone.utc + )