feat(supersession): the relation, and the dead column that stood where it should
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / Python lint (push) Successful in 3s
CI & Build / TypeScript typecheck (push) Successful in 40s
CI & Build / Python tests (push) Failing after 32s
CI & Build / Build & push image (push) Skipped
CI & Build / integration (push) Successful in 25s
CI & Build / Plugin hooks (push) Successful in 13s
CI & Build / Python lint (push) Successful in 3s
CI & Build / TypeScript typecheck (push) Successful in 40s
CI & Build / Python tests (push) Failing after 32s
CI & Build / Build & push image (push) Skipped
CI & Build / integration (push) Successful in 25s
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
This commit is contained in:
@@ -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")
|
||||
@@ -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;
|
||||
|
||||
@@ -41,7 +41,6 @@ const toast = useToastStore();
|
||||
const title = ref("");
|
||||
const body = ref("");
|
||||
const description = ref("");
|
||||
const consolidatedAt = ref<string | null>(null);
|
||||
const tags = ref<string[]>([]);
|
||||
const status = ref<TaskStatus>("todo");
|
||||
const priority = ref<TaskPriority>("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;
|
||||
}
|
||||
</style>
|
||||
@@ -365,13 +365,6 @@ const subTaskProgress = computed(() => {
|
||||
<p class="goal-text">{{ store.currentTask.description }}</p>
|
||||
</div>
|
||||
|
||||
<div
|
||||
v-if="store.currentTask.consolidated_at"
|
||||
class="auto-summary-banner"
|
||||
>
|
||||
<span class="auto-summary-icon" aria-hidden="true">✦</span>
|
||||
Auto-summarized from work logs.
|
||||
</div>
|
||||
|
||||
<div
|
||||
class="body prose"
|
||||
@@ -771,17 +764,4 @@ const subTaskProgress = computed(() => {
|
||||
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;
|
||||
}
|
||||
</style>
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
}
|
||||
@@ -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]:
|
||||
|
||||
Reference in New Issue
Block a user