From 45c6b1c88a5add1b49900c77344259b58a2a4d57 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 7 Aug 2026 21:40:51 -0400 Subject: [PATCH] feat(supersession): the relation, and the dead column that stood where it should MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 1 of #278. Structure only — nothing reads or writes the new table yet. Old records outrank newer ones on the same subject because a similarity score cannot tell time. A note that accurately described how something worked in June is still accurate ABOUT June; it is just no longer the answer. Nothing recorded that, so nothing could act on it. `note_supersessions(superseder_id, superseded_id)`. The claim points FORWARD — the newer record names what it overtakes — because the older one cannot know it has been overtaken; asking it to record its own obsolescence is asking it to predict the future. A table rather than a column because the relation is genuinely many-to-many and partial, and both directions are hot: superseded_id answers "has this been overtaken?" at ranking time, superseder_id answers "what does this replace?" in a record view. An array column serves one and not the other. CASCADE is safe because trashing is not a delete — trash_svc stamps deleted_at, so a trashed note keeps its claims and restore brings them back. It fires only on purge_trash, where a claim about the row would be unactionable anyway. A CHECK rejects self-supersession, which under flat demotion would let a record demote itself. ## consolidated_at, and what it actually was Dropped. Written by nothing while serialised into every note and task payload as null — and worse, it implied a capability. The survey (#2483) read it as note consolidation modelled and abandoned. That was wrong, and the frontend is what says so: `TaskViewerView` rendered "✦ Auto-summarized from work logs" gated on this column. It is a survivor of the pre-pivot auto-summary subsystem (migration 0030), whose own column #599 removed. Not an unbuilt feature — an outlived one. So four more remnants went with it: the banner, its CSS, a `consolidatedAt` ref in TaskEditorView assigned and never read, and `.auto-summary-banner-editor` styling with zero template usage. That last one is presence-without-reference in the same family as the column itself. Dropped rather than repurposed for supersession, and the distinction is the point: consolidation folds records into one survivor and destroys the originals. Supersession is the opposite — both survive, the older ranks behind. Smuggling one in under a column named for the other would bury that in schema. ## The hard delete_note Removed, with a comment where it stood. Zero callers, and the danger was never that it ran — it is that it was findable by name. Someone wanting to delete a note greps `delete_note`, finds a function in the notes service with exactly the right signature, and permanently destroys a record every path downstream expects to be recoverable. The MCP tool of the same name already went through trash_svc; only the service function was the trap. Refs #278, #2483 --- alembic/versions/0076_note_supersessions.py | 115 ++++++++++++++++++++ frontend/src/types/note.ts | 1 - frontend/src/views/TaskEditorView.vue | 20 ---- frontend/src/views/TaskViewerView.vue | 20 ---- src/scribe/models/__init__.py | 1 + src/scribe/models/note.py | 6 - src/scribe/models/note_supersession.py | 70 ++++++++++++ src/scribe/services/notes.py | 21 ++-- 8 files changed, 196 insertions(+), 58 deletions(-) create mode 100644 alembic/versions/0076_note_supersessions.py create mode 100644 src/scribe/models/note_supersession.py diff --git a/alembic/versions/0076_note_supersessions.py b/alembic/versions/0076_note_supersessions.py new file mode 100644 index 0000000..8fd6b86 --- /dev/null +++ b/alembic/versions/0076_note_supersessions.py @@ -0,0 +1,115 @@ +"""note_supersessions; drop the never-written notes.consolidated_at + +Revision ID: 0076 +Revises: 0075 +Create Date: 2026-08-07 + +Step 1 of milestone #278. Structure only — nothing reads or writes the new +table yet, and nothing behaves differently after this runs. + +## What the table is for + +Old records outrank newer ones on the same subject, because a similarity score +cannot tell time. A note that accurately described how something worked in June +is still accurate ABOUT June; it is just no longer the answer. Nothing recorded +that, so nothing could act on it. + +The claim points FORWARD — the newer record names what it overtakes — because +the older one cannot know it has been overtaken. Many-to-many and partial: a +note may supersede parts of several others and be overtaken piecemeal by +several later ones, which is why this is a table rather than a column. Both +directions are queried: `superseded_id` answers "has this been overtaken?" at +ranking time, `superseder_id` answers "what does this replace?" in a record +view. An array column could serve one and not the other. + +CASCADE on both sides is safe because trashing is not a delete: `trash_svc` +stamps `deleted_at`, so a trashed note keeps its claims and `restore` brings +them back. The cascade fires only on `purge_trash`, where the row genuinely +goes — and a claim about a row that no longer exists is not actionable. + +## What is being dropped, and why now + +`notes.consolidated_at` was written by NOTHING — no service, no route, no tool +— while being serialised into every note and task payload as `null`. It cost a +column, a line in every response, and worse: it IMPLIED a capability. A reader +reasonably concludes notes can be consolidated and this records when. + +That reading was reasonable precisely because merge/unmerge exists for snippets +and not for notes, so the column looked like the notes-side half of that +feature, modelled and abandoned. + +It is dropped rather than repurposed for supersession, and the distinction is +the point (#2483): consolidation folds several records into one survivor and +destroys the originals. Merging two snippets is lossless — one helper, several +call sites. Folding two dev-logs means writing a summary and losing what each +actually said. Supersession is the opposite act: both records survive, and the +older one is merely ranked behind. Smuggling one in under a column named for +the other would have buried that difference in schema. + +## Downgrade + +Re-adds `consolidated_at` nullable, which is how it lived — so downgrade +restores the shape, not the (nonexistent) data. Drops the table; any recorded +supersession claims are lost, which costs ranking its input and nothing else, +since no note's own content depends on them. +""" +import sqlalchemy as sa +from alembic import op + +revision = "0076" +down_revision = "0075" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.create_table( + "note_supersessions", + sa.Column("id", sa.Integer, primary_key=True), + sa.Column( + "superseder_id", + sa.Integer, + sa.ForeignKey("notes.id", ondelete="CASCADE"), + nullable=False, + ), + sa.Column( + "superseded_id", + sa.Integer, + sa.ForeignKey("notes.id", ondelete="CASCADE"), + nullable=False, + ), + sa.Column( + "created_at", + sa.DateTime(timezone=True), + server_default=sa.text("now()"), + nullable=False, + ), + sa.UniqueConstraint( + "superseder_id", "superseded_id", name="uq_note_supersessions_pair" + ), + # Declaring that a note supersedes ITSELF is meaningless, and under flat + # demotion it would demote a record on its own authority. Refused in the + # service too, with a message — this is the backstop that holds when + # something writes rows directly. + sa.CheckConstraint( + "superseder_id <> superseded_id", name="ck_note_supersessions_not_self" + ), + ) + op.create_index( + "ix_note_supersessions_superseder", "note_supersessions", ["superseder_id"] + ) + op.create_index( + "ix_note_supersessions_superseded", "note_supersessions", ["superseded_id"] + ) + + op.drop_column("notes", "consolidated_at") + + +def downgrade() -> None: + op.add_column( + "notes", + sa.Column("consolidated_at", sa.DateTime(timezone=True), nullable=True), + ) + op.drop_index("ix_note_supersessions_superseded", table_name="note_supersessions") + op.drop_index("ix_note_supersessions_superseder", table_name="note_supersessions") + op.drop_table("note_supersessions") diff --git a/frontend/src/types/note.ts b/frontend/src/types/note.ts index 01f269f..64105e4 100644 --- a/frontend/src/types/note.ts +++ b/frontend/src/types/note.ts @@ -10,7 +10,6 @@ export interface Note { title: string; body: string; description: string | null; - consolidated_at: string | null; tags: string[]; parent_id: number | null; parent_title?: string | null; diff --git a/frontend/src/views/TaskEditorView.vue b/frontend/src/views/TaskEditorView.vue index b0d1932..ae1cacf 100644 --- a/frontend/src/views/TaskEditorView.vue +++ b/frontend/src/views/TaskEditorView.vue @@ -41,7 +41,6 @@ const toast = useToastStore(); const title = ref(""); const body = ref(""); const description = ref(""); -const consolidatedAt = ref(null); const tags = ref([]); const status = ref("todo"); const priority = ref("none"); @@ -303,7 +302,6 @@ onMounted(async () => { title.value = store.currentTask.title; body.value = store.currentTask.body; description.value = store.currentTask.description ?? ""; - consolidatedAt.value = store.currentTask.consolidated_at ?? null; tags.value = [...(store.currentTask.tags || [])]; status.value = store.currentTask.status as TaskStatus; priority.value = store.currentTask.priority as TaskPriority; @@ -1063,22 +1061,4 @@ useEditorGuards(dirty, save); border-color: var(--color-primary); } -/* ── Auto-summary banner + re-consolidate button ─────────────────────────── */ -.auto-summary-banner-editor { - display: flex; - align-items: center; - gap: 0.6rem; - padding: 0.45rem 0.7rem; - margin-bottom: 0.5rem; - font-size: 0.82rem; - font-style: italic; - color: var(--color-text-muted); - background: rgba(99, 102, 241, 0.06); - border-left: 2px solid var(--color-primary); - border-radius: var(--radius-sm); -} -.auto-summary-banner-editor .auto-summary-icon { - color: var(--color-primary); - font-style: normal; -} \ No newline at end of file diff --git a/frontend/src/views/TaskViewerView.vue b/frontend/src/views/TaskViewerView.vue index a05e60f..dc98184 100644 --- a/frontend/src/views/TaskViewerView.vue +++ b/frontend/src/views/TaskViewerView.vue @@ -365,13 +365,6 @@ const subTaskProgress = computed(() => {

{{ store.currentTask.description }}

-
- - Auto-summarized from work logs. -
{ color: var(--color-text); white-space: pre-wrap; } -.auto-summary-banner { - display: flex; - align-items: center; - gap: 0.5rem; - font-size: 0.78rem; - font-style: italic; - color: var(--color-text-muted); - margin: 0 0 0.75rem; -} -.auto-summary-icon { - color: var(--color-primary); - font-size: 0.85rem; -} diff --git a/src/scribe/models/__init__.py b/src/scribe/models/__init__.py index 0135af2..7d3cf92 100644 --- a/src/scribe/models/__init__.py +++ b/src/scribe/models/__init__.py @@ -33,6 +33,7 @@ from scribe.models.milestone import Milestone # noqa: E402, F401 from scribe.models.task_log import TaskLog # noqa: E402, F401 from scribe.models.note_draft import NoteDraft # noqa: E402, F401 from scribe.models.note_version import NoteVersion # noqa: E402, F401 +from scribe.models.note_supersession import NoteSupersession # noqa: E402, F401 from scribe.models.group import Group, GroupMembership # noqa: E402, F401 from scribe.models.share import NoteShare, ProjectShare # noqa: E402, F401 from scribe.models.notification import Notification # noqa: E402, F401 diff --git a/src/scribe/models/note.py b/src/scribe/models/note.py index 1b85701..2ba05f3 100644 --- a/src/scribe/models/note.py +++ b/src/scribe/models/note.py @@ -33,9 +33,6 @@ class Note(Base, TimestampMixin, SoftDeleteMixin): title: Mapped[str] = mapped_column(Text, default="") body: Mapped[str] = mapped_column(Text, default="") description: Mapped[str | None] = mapped_column(Text, nullable=True) - consolidated_at: Mapped[datetime | None] = mapped_column( - DateTime(timezone=True), nullable=True - ) tags: Mapped[list[str]] = mapped_column(ARRAY(Text), default=list) parent_id: Mapped[int | None] = mapped_column( Integer, ForeignKey("notes.id", ondelete="SET NULL"), nullable=True @@ -101,9 +98,6 @@ class Note(Base, TimestampMixin, SoftDeleteMixin): "title": self.title, "body": self.body, "description": self.description, - "consolidated_at": ( - self.consolidated_at.isoformat() if self.consolidated_at else None - ), "tags": self.tags or [], "parent_id": self.parent_id, "arose_from_id": self.arose_from_id, diff --git a/src/scribe/models/note_supersession.py b/src/scribe/models/note_supersession.py new file mode 100644 index 0000000..127ccec --- /dev/null +++ b/src/scribe/models/note_supersession.py @@ -0,0 +1,70 @@ +from sqlalchemy import ForeignKey, Index, Integer, UniqueConstraint +from sqlalchemy.orm import Mapped, mapped_column + +from scribe.models import Base +from scribe.models.base import CreatedAtMixin + + +class NoteSupersession(Base, CreatedAtMixin): + """A newer record's claim that it has overtaken an older one. + + WHY THE RELATION POINTS FORWARD + + The note being WRITTEN declares what it supersedes. The old record cannot + know it has been overtaken — asking it to record its own obsolescence is + asking it to predict the future. So the claim is made by the party that has + the knowledge, and the demotion is derived from the far end. + + WHY A TABLE RATHER THAN A COLUMN + + It is genuinely many-to-many and partial: one note may supersede parts of + several others, and a note may be overtaken piecemeal by several later ones. + Both directions are queried and neither is rare — + `superseded_id` answers the ranking question ("has this been overtaken?"), + `superseder_id` answers the record view ("what does this replace?"). An + array column on `notes` could be indexed for one and not the other. + + WHAT IT MEANS, AND WHAT IT DOES NOT + + A claim, never a proof. Supersession DEMOTES a record in ranked retrieval; + it does not assert the older record was wrong, and it never hides it. A note + that accurately described how something worked in June is still accurate + about June — it is just no longer the answer to "how does this work". + + CASCADE IS SAFE HERE BECAUSE TRASHING IS NOT A DELETE + + `trash_svc` stamps `deleted_at` (an UPDATE), so a trashed note keeps its + claims and `restore` brings them back intact. The cascade fires only on + `purge_trash`, where the row genuinely goes — and a supersession claim about + a row that no longer exists is not a fact anyone can act on. + """ + + __tablename__ = "note_supersessions" + + id: Mapped[int] = mapped_column(primary_key=True) + # The newer record, making the claim. + superseder_id: Mapped[int] = mapped_column( + Integer, ForeignKey("notes.id", ondelete="CASCADE") + ) + # The older record, demoted by it. + superseded_id: Mapped[int] = mapped_column( + Integer, ForeignKey("notes.id", ondelete="CASCADE") + ) + + __table_args__ = ( + UniqueConstraint( + "superseder_id", "superseded_id", name="uq_note_supersessions_pair" + ), + # Both directions indexed — see the class docstring for why neither is + # the rare one. + Index("ix_note_supersessions_superseder", "superseder_id"), + Index("ix_note_supersessions_superseded", "superseded_id"), + ) + + def to_dict(self) -> dict: + return { + "id": self.id, + "superseder_id": self.superseder_id, + "superseded_id": self.superseded_id, + "created_at": self.created_at.isoformat() if self.created_at else None, + } diff --git a/src/scribe/services/notes.py b/src/scribe/services/notes.py index 698958d..649a28d 100644 --- a/src/scribe/services/notes.py +++ b/src/scribe/services/notes.py @@ -373,17 +373,16 @@ async def update_note(user_id: int, note_id: int, **fields: object) -> Note | No return note -async def delete_note(user_id: int, note_id: int) -> bool: - async with async_session() as session: - result = await session.execute( - select(Note).where(Note.id == note_id, Note.user_id == user_id) - ) - note = result.scalars().first() - if note is None: - return False - await session.delete(note) - await session.commit() - return True +# A hard `delete_note(user_id, note_id)` lived here with ZERO callers, and was +# removed with #278 step 1. It is recorded rather than silently dropped because +# the danger was never that it ran — it is that it was findable by name. Someone +# wanting to delete a note greps `delete_note`, finds a function in the notes +# service with exactly the right signature, and permanently destroys a record +# every path downstream expects to be recoverable. +# +# The delete path is `trash_svc.delete`, which soft-deletes an entity AND its +# descendants under one batch_id so `restore(batch)` works. `purge_trash` owns +# permanent deletion. Both are reachable; neither is spelled `delete_note`. async def get_all_tags(user_id: int, q: str | None = None) -> list[str]: