diff --git a/alembic/versions/0023_note_link_target_id.py b/alembic/versions/0023_note_link_target_id.py new file mode 100644 index 0000000..4a21a9d --- /dev/null +++ b/alembic/versions/0023_note_link_target_id.py @@ -0,0 +1,67 @@ +"""note_links.target_id — resolve [[links]] to a note, not to a string (M13 step 1) + +Revision ID: 0023 +Revises: 0022 +Create Date: 2026-08-22 + +A wiki-link stored only as normalized TEXT means a note's name IS the edge: rename +the note and every inbound link stops matching. The old answer was to rewrite the +`[[Old Name]]` text inside every note that linked to it — workable while an explicit +title existed to hold still, untenable once a note's name is just its first body +line (M13). + +`target_norm` stays: it is what an UNRESOLVED link carries, since linking to a note +that doesn't exist yet is a supported way to create one. + +The backfill is safe to run bluntly because note_links is DERIVED data — every row +is recomputed from the source body on the next save regardless. +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = "0023" +down_revision = "0022" +branch_labels = None +depends_on = None + + +def upgrade() -> None: + op.add_column( + "note_links", + sa.Column("target_id", postgresql.UUID(as_uuid=True), nullable=True), + ) + op.create_foreign_key( + "fk_note_links_target", + "note_links", + "notes", + ["target_id"], + ["id"], + # A deleted target un-resolves its inbound links rather than deleting them: + # the link text is still in the source's body, and it should read as pointing + # at something that isn't there — which is also what lets it re-resolve if a + # note of that name appears again. + ondelete="SET NULL", + ) + op.create_index("ix_note_links_target_id", "note_links", ["target_id"]) + + # Resolve what can be resolved right now, scoped to the source's owner so a link + # can never bind to another user's note. + op.execute( + """ + UPDATE note_links AS nl + SET target_id = t.id + FROM notes AS src, notes AS t + WHERE nl.source_id = src.id + AND t.owner_id = src.owner_id + AND t.deleted_at IS NULL + AND lower(btrim(t.display_title)) = nl.target_norm + AND t.id <> src.id + """ + ) + + +def downgrade() -> None: + op.drop_index("ix_note_links_target_id", table_name="note_links") + op.drop_constraint("fk_note_links_target", "note_links", type_="foreignkey") + op.drop_column("note_links", "target_id") diff --git a/frontend/src/components/MarkdownInline.vue b/frontend/src/components/MarkdownInline.vue index 762db0a..1df9f21 100644 --- a/frontend/src/components/MarkdownInline.vue +++ b/frontend/src/components/MarkdownInline.vue @@ -1,26 +1,49 @@ @@ -35,7 +58,7 @@ async function follow(title: string) { class="cursor-pointer font-medium text-brand-700 underline-offset-2 hover:underline dark:text-brand" @click.stop="follow(t.value)" @keydown.enter.stop.prevent="follow(t.value)" - >{{ t.value }}{{ label(t.value) }}{{ t.value }}{{ t.value }}(); +// `links` is the owning note's resolved [[links]], passed straight through to every +// inline run — only MarkdownInline uses it, but only this component knows the note. +const props = defineProps<{ text: string; links?: NoteLinkRef[] }>(); const blocks = computed(() => parseMarkdown(props.text)); diff --git a/frontend/src/components/NoteCard.vue b/frontend/src/components/NoteCard.vue index 6f95476..9ef780d 100644 --- a/frontend/src/components/NoteCard.vue +++ b/frontend/src/components/NoteCard.vue @@ -237,7 +237,7 @@ onBeforeUnmount(() => document.removeEventListener("mousedown", onDocMousedown)) {{ note.title }}
- +

Empty note diff --git a/frontend/src/components/NoteEditor.vue b/frontend/src/components/NoteEditor.vue index ece6d59..aa36885 100644 --- a/frontend/src/components/NoteEditor.vue +++ b/frontend/src/components/NoteEditor.vue @@ -266,7 +266,13 @@ onMounted(async () => { }); // ---- outgoing links (edit mode) ---- +// +// Two sources, in order. The note's SAVED links carry the server's binding, so a +// target that has since been renamed still resolves and is listed under its current +// name. A link just typed into the textarea has no saved row yet, and the name index +// is the best that can be said about it until the note is saved. const outgoingLinks = computed(() => { + const boundByNorm = new Map((props.note?.links ?? []).map((l) => [l.norm, l])); const re = /\[\[([^[\]]+)\]\]/g; const seen = new Set(); const out: { title: string; id: string | null }[] = []; @@ -276,7 +282,8 @@ const outgoingLinks = computed(() => { const key = t.toLowerCase(); if (t && !seen.has(key)) { seen.add(key); - out.push({ title: t, id: titles.resolve(t)?.id ?? null }); + const hit = boundByNorm.get(key); + out.push(hit ? { title: hit.title, id: hit.id } : { title: t, id: titles.resolve(t)?.id ?? null }); } } return out; diff --git a/frontend/src/stores/notes.ts b/frontend/src/stores/notes.ts index a3fab2e..2891e89 100644 --- a/frontend/src/stores/notes.ts +++ b/frontend/src/stores/notes.ts @@ -63,6 +63,22 @@ export interface NoteRevision { created_at: string | null; } +// One resolved [[wiki-link]] out of a note: the normalized text as WRITTEN, and the +// note it actually points at with that note's name as it stands NOW. +// +// The server sends this because the client can no longer work it out. Resolution used +// to be a name lookup in the titles index, which only held together because renaming +// a note rewrote the link text inside every note that linked to it. Links are bound +// by id now and bodies are left alone, so the written text can name something the +// target is no longer called — and only the server holds the binding. +export interface NoteLinkRef { + /** The link text as written, trimmed and lowercased — the key a token matches on. */ + norm: string; + id: string; + /** The target's CURRENT name, which is what gets rendered. */ + title: string; +} + export interface Note { id: string; title: string | null; @@ -85,6 +101,11 @@ export interface Note { items: ChecklistItem[]; attachments: Attachment[]; previews: LinkPreview[]; + // Absent offline: the desktop's local store derives links at query time and has no + // resolution to send. Rendering falls back to the titles index there, which is + // exactly right for a store where nothing else can have renamed the target behind + // this client's back. + links?: NoteLinkRef[]; created_at: string | null; updated_at: string | null; } diff --git a/src/thoughtsync/graph.py b/src/thoughtsync/graph.py index b6ecde3..8470e91 100644 --- a/src/thoughtsync/graph.py +++ b/src/thoughtsync/graph.py @@ -1,7 +1,7 @@ from __future__ import annotations from quart import Blueprint, g, jsonify -from sqlalchemy import func, select +from sqlalchemy import and_, func, or_, select from sqlalchemy.orm import aliased from .auth import login_required @@ -33,11 +33,23 @@ async def get_graph(): """ source = aliased(Note) target = aliased(Note) + # A link joins on the note it was BOUND to, and only falls back to matching by + # name where it was never bound — a forward link written before its target + # existed. Name-matching alone is what used to make a rename break the graph. edge_stmt = ( select(source.id, target.id) .select_from(NoteLink) .join(source, source.id == NoteLink.source_id) - .join(target, func.lower(func.trim(target.display_title)) == NoteLink.target_norm) + .join( + target, + or_( + target.id == NoteLink.target_id, + and_( + NoteLink.target_id.is_(None), + func.lower(func.trim(target.display_title)) == NoteLink.target_norm, + ), + ), + ) .where( source.owner_id == g.user_id, source.deleted_at.is_(None), diff --git a/src/thoughtsync/models/note_link.py b/src/thoughtsync/models/note_link.py index 0a195e3..fa91dc1 100644 --- a/src/thoughtsync/models/note_link.py +++ b/src/thoughtsync/models/note_link.py @@ -10,14 +10,38 @@ from . import Base class NoteLink(Base): - """A [[wiki-link]] from a source note to a target title (normalized). Resolved - to a target note by matching target_norm against lower(trim(note.title)).""" + """A [[wiki-link]] from a source note to another note. + + Two target columns, and the pair is the point: + + - ``target_id`` — the note this link actually points at, bound when the link was + written. This is what makes a link survive its target being RENAMED. A note's + name is derived from its first body line, so without an id the name is the + edge, and editing that line would silently break every inbound link (or, in the + older design, force a rewrite of every linking note's body). + - ``target_norm`` — the normalized link text, always stored. It is what an + UNRESOLVED link carries: `[[a note that doesn't exist yet]]` is a supported way + to create one, so a link has to be able to name a target that isn't there. + + Resolution reads the id first and falls back to matching the norm against + ``notes.display_title``, which is how a forward link connects the moment its + target appears. ``_claim_unresolved_links`` then binds the id, so the fallback is + a transitional state rather than a permanent one. + """ __tablename__ = "note_links" - __table_args__ = (Index("ix_note_links_target", "target_norm"),) + __table_args__ = ( + Index("ix_note_links_target", "target_norm"), + Index("ix_note_links_target_id", "target_id"), + ) id: Mapped[uuid.UUID] = mapped_column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) source_id: Mapped[uuid.UUID] = mapped_column( UUID(as_uuid=True), ForeignKey("notes.id", ondelete="CASCADE"), nullable=False ) + # SET NULL rather than CASCADE: deleting the target un-resolves the link, it does + # not delete it. The link text is still sitting in the source's body. + target_id: Mapped[uuid.UUID | None] = mapped_column( + UUID(as_uuid=True), ForeignKey("notes.id", ondelete="SET NULL"), nullable=True + ) target_norm: Mapped[str] = mapped_column(Text(), nullable=False) diff --git a/src/thoughtsync/notes/__init__.py b/src/thoughtsync/notes/__init__.py index 5ec4054..b0f8c9f 100644 --- a/src/thoughtsync/notes/__init__.py +++ b/src/thoughtsync/notes/__init__.py @@ -19,7 +19,7 @@ import zipfile from datetime import datetime, timedelta, timezone from quart import Response, g, jsonify, request, send_file -from sqlalchemy import case, func, literal_column, select +from sqlalchemy import and_, case, func, literal_column, or_, select from ..acl import visible_to_user from ..auth import login_required @@ -67,11 +67,10 @@ from .import_export import ( ) from .links import ( _reconcile_tags, - _rename_inbound_links, + _claim_unresolved_links, _rewrite_links, parse_link_titles, parse_tags, - rewrite_link_title, ) from .recurrence import REMINDER_RECURRENCES, next_occurrence, normalize_recurrence from .serialize import _items_for_notes, _labels_for_notes, _serialize_note, _serialize_notes @@ -83,12 +82,11 @@ __all__ = [ "parse_list_items", "parse_tags", "parse_link_titles", - "rewrite_link_title", "normalize_color", "normalize_recurrence", "next_occurrence", "_reconcile_tags", - "_rename_inbound_links", + "_claim_unresolved_links", "_rewrite_links", "_serialize_notes", "_escape_like", @@ -435,15 +433,20 @@ async def note_backlinks(note_id: str): ) if note is None: return not_found() - if not note.display_title: - return jsonify({"backlinks": []}) - norm = note.display_title.strip().lower() + # Bound links (target_id) OR unbound ones still naming this note. The second + # half is what catches a link written before this note existed and not yet + # claimed; without it a forward link would go quiet until its source is next + # saved. + norm = (note.display_title or "").strip().lower() + matches = NoteLink.target_id == nid + if norm: + matches = or_(matches, and_(NoteLink.target_id.is_(None), NoteLink.target_norm == norm)) sources = ( await db.scalars( select(Note) .join(NoteLink, NoteLink.source_id == Note.id) .where( - NoteLink.target_norm == norm, + matches, Note.owner_id == g.user_id, Note.deleted_at.is_(None), Note.id != nid, @@ -524,6 +527,9 @@ async def create_note(): db.add(NoteItem(note_id=note.id, text=text, position=pos)) await _rewrite_links(db, note) await _reconcile_tags(db, note) + # A new note may be exactly what earlier `[[links]]` were pointing at — the + # create-by-linking flow writes the link first and the note second. + await _claim_unresolved_links(db, note) await db.commit() await db.refresh(note) return jsonify(await _serialize_note(db, note)), 201 @@ -587,12 +593,12 @@ async def update_note(note_id: str): if "body" in data: await _rewrite_links(db, note) await _reconcile_tags(db, note) - # The display NAME changing — via an explicit title OR the first body line — - # repoints inbound [[Old Name]] references so backlinks survive (skip pure - # case/whitespace changes, which still resolve). + # A rename no longer touches anything else's TEXT. Inbound links already hold + # this note's id, so they follow it automatically; all that is left is to + # adopt any still-unresolved link that was waiting for this name. new_display = note.display_title - if old_display and new_display and old_display.strip().lower() != new_display.strip().lower(): - await _rename_inbound_links(db, note, old_display, new_display) + if old_display != new_display: + await _claim_unresolved_links(db, note) # Version history: snapshot the PRE-edit title+body whenever either changed. if note.title != old_title or note.body != old_body: db.add(NoteRevision(note_id=note.id, title=old_title, body=old_body)) @@ -653,8 +659,8 @@ async def restore_revision(note_id: str, rev_id: str): await _rewrite_links(db, note) await _reconcile_tags(db, note) new_display = note.display_title - if old_display and new_display and old_display.strip().lower() != new_display.strip().lower(): - await _rename_inbound_links(db, note, old_display, new_display) + if old_display != new_display: + await _claim_unresolved_links(db, note) await db.commit() await db.refresh(note) return jsonify(await _serialize_note(db, note)) diff --git a/src/thoughtsync/notes/links.py b/src/thoughtsync/notes/links.py index 7c31071..5fb4128 100644 --- a/src/thoughtsync/notes/links.py +++ b/src/thoughtsync/notes/links.py @@ -5,7 +5,8 @@ from __future__ import annotations import re -from sqlalchemy import delete, func, select +from sqlalchemy import delete, func, select, update +from sqlalchemy.orm import aliased from ..models.label import Label, NoteLabel from ..models.note import Note @@ -49,11 +50,65 @@ def parse_link_titles(body: str | None) -> list[str]: return out +async def _resolve_target(db, owner_id, norm: str, exclude_id=None): + """The owner's note currently NAMED `norm`, or None. + + Owner-scoped so a link can never bind to someone else's note, and self-excluded + so a note that opens with its own name doesn't link to itself. + """ + stmt = select(Note.id).where( + Note.owner_id == owner_id, + Note.deleted_at.is_(None), + func.lower(func.trim(Note.display_title)) == norm, + ) + if exclude_id is not None: + stmt = stmt.where(Note.id != exclude_id) + return await db.scalar(stmt) + + async def _rewrite_links(db, note: Note) -> None: - """Replace a note's outgoing wiki-links from its current body.""" + """Replace a note's outgoing wiki-links from its current body. + + Each link is bound to the target's ID where one exists under that name right now. + That binding is what survives the target being renamed later; the norm is kept + either way, so a link to a note that doesn't exist yet is still recorded and can + resolve when it does. + """ await db.execute(delete(NoteLink).where(NoteLink.source_id == note.id)) for norm in parse_link_titles(note.body): - db.add(NoteLink(source_id=note.id, target_norm=norm)) + target_id = await _resolve_target(db, note.owner_id, norm, exclude_id=note.id) + db.add(NoteLink(source_id=note.id, target_norm=norm, target_id=target_id)) + + +async def _claim_unresolved_links(db, note: Note) -> None: + """Bind still-unresolved links that name this note to it. + + Called when a note's display name changes or a note is created. Two cases, one + mechanism: someone wrote `[[groceries]]` before any note was called that, or a + note has just been renamed INTO a name that other notes were already pointing at. + + This is what replaced `_rename_inbound_links`, and the difference is the whole + point of the change: that function edited the BODIES of other people's notes to + keep their link text matching. This touches only link rows. A note's text is + never modified by something happening to a different note. + """ + norm = (note.display_title or "").strip().lower() + if not norm: + return + source = aliased(Note) + unresolved = ( + select(NoteLink.id) + .join(source, source.id == NoteLink.source_id) + .where( + NoteLink.target_id.is_(None), + NoteLink.target_norm == norm, + source.owner_id == note.owner_id, + source.id != note.id, + ) + ) + await db.execute( + update(NoteLink).where(NoteLink.id.in_(unresolved.scalar_subquery())).values(target_id=note.id) + ) async def _find_or_create_label(db, owner_id, name: str): @@ -89,38 +144,3 @@ async def _reconcile_tags(db, note: Note) -> None: if lid not in attached_ids: db.add(NoteLabel(note_id=note.id, label_id=lid, via_tag=True)) attached_ids.add(lid) - - -def rewrite_link_title(body: str | None, old_norm: str, new_title: str) -> str: - """Repoint every [[token]] whose normalized form == old_norm to [[new_title]].""" - if not body: - return body or "" - - def _sub(match: re.Match) -> str: - return f"[[{new_title}]]" if match.group(1).strip().lower() == old_norm else match.group(0) - - return _LINK_RE.sub(_sub, body) - - -async def _rename_inbound_links(db, renamed: Note, old_title: str, new_title: str) -> None: - """Rewrite [[old title]] references (and their link rows) in every note that - links to the renamed note, so its backlinks survive the title change.""" - old_norm = old_title.strip().lower() - sources = ( - await db.scalars( - select(Note) - .join(NoteLink, NoteLink.source_id == Note.id) - .where( - NoteLink.target_norm == old_norm, - Note.owner_id == renamed.owner_id, - Note.deleted_at.is_(None), - ) - ) - ).all() - seen: set = set() - for source in sources: - if source.id in seen: - continue - seen.add(source.id) - source.body = rewrite_link_title(source.body, old_norm, new_title) - await _rewrite_links(db, source) diff --git a/src/thoughtsync/notes/serialize.py b/src/thoughtsync/notes/serialize.py index cceff35..139649a 100644 --- a/src/thoughtsync/notes/serialize.py +++ b/src/thoughtsync/notes/serialize.py @@ -1,14 +1,16 @@ -"""Note serialization — turn a Note (+ its labels/items/attachments/previews) into -the JSON dict the API returns. The bulk loaders (`*_for_notes`) fetch each child +"""Note serialization — turn a Note (+ its labels/items/attachments/previews/links) +into the JSON dict the API returns. The bulk loaders (`*_for_notes`) fetch each child collection for a batch of notes in one query, so list endpoints avoid N+1s.""" from __future__ import annotations -from sqlalchemy import select +from sqlalchemy import and_, func, or_, select +from sqlalchemy.orm import aliased from ..models.label import Label, NoteLabel from ..models.note import Note from ..models.note_attachment import NoteAttachment from ..models.note_item import NoteItem +from ..models.note_link import NoteLink from ..models.note_link_preview import NoteLinkPreview @@ -104,6 +106,64 @@ async def _previews_for_notes(db, note_ids: list) -> dict: return result +async def _links_for_notes(db, note_ids: list) -> dict: + """Map note_id -> [{norm, id, title}] for each note's RESOLVED outgoing links. + + The client cannot work this out for itself any more, and that is deliberate. It + used to resolve `[[text]]` by looking the text up in a client-side name index, + which only worked because a rename rewrote the text in every linking note. Now + that a link is bound to an id and the text is left alone, the stored text can name + something the target is no longer called — so the server, which holds the binding, + is the only place that knows where a link goes. + + `title` is the target's name RIGHT NOW, so a renamed note reads correctly + everywhere it is linked from without a single body having been edited. + + Unresolved links are simply absent: the client renders those as the + create-on-click affordance it already has. + """ + if not note_ids: + return {} + source = aliased(Note) + target = aliased(Note) + rows = ( + await db.execute( + select(NoteLink.source_id, NoteLink.target_norm, target.id, target.display_title) + .select_from(NoteLink) + .join(source, source.id == NoteLink.source_id) + .join( + target, + or_( + target.id == NoteLink.target_id, + and_( + NoteLink.target_id.is_(None), + func.lower(func.trim(target.display_title)) == NoteLink.target_norm, + ), + ), + ) + .where( + NoteLink.source_id.in_(note_ids), + target.deleted_at.is_(None), + # Owner-scoped, and NOT optional. A bound target_id was resolved + # owner-scoped when it was written, but the name fallback matches on + # display_title alone — without this, two users who both have a note + # called "Groceries" would leak each other's note id and name through + # an unresolved link. (Rule 47.) + target.owner_id == source.owner_id, + ) + ) + ).all() + result: dict = {} + for source_id, norm, target_id, title in rows: + bucket = result.setdefault(source_id, []) + # The name-fallback join can produce more than one candidate for the same + # text; first one wins, deterministically enough for a display hint. + if any(link["norm"] == norm for link in bucket): + continue + bucket.append({"norm": norm, "id": str(target_id), "title": title}) + return result + + async def _serialize_note(db, note: Note) -> dict: data = note.serialize() labels = await _labels_for_notes(db, [note.id]) @@ -114,6 +174,8 @@ async def _serialize_note(db, note: Note) -> dict: data["attachments"] = attachments.get(note.id, []) previews = await _previews_for_notes(db, [note.id]) data["previews"] = previews.get(note.id, []) + links = await _links_for_notes(db, [note.id]) + data["links"] = links.get(note.id, []) return data @@ -123,6 +185,7 @@ async def _serialize_notes(db, notes: list) -> list: items_map = await _items_for_notes(db, ids) attach_map = await _attachments_for_notes(db, ids) preview_map = await _previews_for_notes(db, ids) + link_map = await _links_for_notes(db, ids) out = [] for n in notes: data = n.serialize() @@ -130,5 +193,6 @@ async def _serialize_notes(db, notes: list) -> list: data["items"] = items_map.get(n.id, []) data["attachments"] = attach_map.get(n.id, []) data["previews"] = preview_map.get(n.id, []) + data["links"] = link_map.get(n.id, []) out.append(data) return out diff --git a/src/thoughtsync/sync.py b/src/thoughtsync/sync.py index f134e07..fad4e81 100644 --- a/src/thoughtsync/sync.py +++ b/src/thoughtsync/sync.py @@ -28,7 +28,7 @@ from .models.note_item import NoteItem from .models.note_revision import NoteRevision from .notes import ( _reconcile_tags, - _rename_inbound_links, + _claim_unresolved_links, _rewrite_links, _serialize_notes, derive_display_title, @@ -295,9 +295,12 @@ async def _apply_note(db, ch: dict) -> dict: await _rewrite_links(db, note) await _reconcile_tags(db, note) await _apply_note_manual_labels(db, note, ch) + # Any change to the name — including a note arriving for the first time, where + # the old name was empty — may be what unresolved inbound links were waiting for. + # Nothing else's body is touched; see _claim_unresolved_links. new_display = note.display_title - if old_display and new_display and old_display.strip().lower() != new_display.strip().lower(): - await _rename_inbound_links(db, note, old_display, new_display) + if old_display != new_display: + await _claim_unresolved_links(db, note) await db.flush() await db.refresh(note, ["sync_revision"]) return { diff --git a/tests/test_notes.py b/tests/test_notes.py index 715309f..86b58d2 100644 --- a/tests/test_notes.py +++ b/tests/test_notes.py @@ -22,7 +22,6 @@ from thoughtsync.notes import ( parse_link_titles, parse_list_items, parse_tags, - rewrite_link_title, ) @@ -128,15 +127,22 @@ def test_parse_link_titles_empty(): assert parse_link_titles("no links here") == [] -def test_rewrite_link_title(): - body = "see [[Alpha]] and [[ alpha ]] and [[Beta]]" - assert rewrite_link_title(body, "alpha", "Gamma") == "see [[Gamma]] and [[Gamma]] and [[Beta]]" +# `rewrite_link_title` and `_rename_inbound_links` are gone (M13 step 1). They kept +# backlinks alive across a rename by editing the [[text]] inside every note that +# linked to the renamed one — which meant one note's edit silently rewrote another's +# words. Links are bound to a note id now, so a rename needs no repair at all. +# +# What replaced them (`_resolve_target`, `_claim_unresolved_links`, and the id-first +# resolution in backlinks / the graph / serialization) is all SQL, and this suite runs +# without a database, so it is deliberately not asserted here. See the task log: that +# behaviour was checked by hand, and this repo has no integration lane to hold it. -def test_rewrite_link_title_noop(): - assert rewrite_link_title("", "alpha", "Gamma") == "" - assert rewrite_link_title(None, "alpha", "Gamma") == "" - assert rewrite_link_title("no links here", "alpha", "Gamma") == "no links here" +def test_parse_link_titles_ignores_nesting(): + # The link regex refuses [ and ] inside a token, so a malformed nest yields the + # inner name rather than something spanning both — worth pinning, since this is + # the string that becomes a link's stored target. + assert parse_link_titles("[[outer [[inner]] ]]") == ["inner"] def test_derive_display_title_explicit_wins():