fix(index): remember former titles so a rename does not orphan inbound links
Link resolution re-reads the link text on every reindex: `[[Old Title]]` is looked up again and the result overwrites the stored edge. So renaming a note did not break anything at the time — it armed a trap. The next time each inbound source was reindexed, for any unrelated reason, its link silently became `unresolved:Old Title`, and `reindex --full` sprang every one of them at once. Nothing in the codebase handled a title change. A new `note_titles` table records every title a note has carried, written in the same transaction as its links. `_resolve` consults it only after the live title and slug lookups miss, so a *new* note claiming a freed title still wins — the link text should name whatever holds that title now, not whatever held it first. Where a title has had several holders, the most recent wins: deterministic, and the least surprising reading. `open()` backfills the table from the titles already in `notes`. Without that an upgraded index only learns a title when the note is next reindexed, and a note that never changes never is — so the first rename after the upgrade would still break its links, which is the whole case this exists to prevent. `remove()` drops a note's titles, so a deleted ULID cannot keep winning resolution for a title nothing holds any more. Reverting the lookup fails the rename and backfill tests; reverting only the backfill fails the backfill test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0139vHnNpS2sLNELjrAHX5Cu
This commit is contained in:
1 parent
79f88fdd51
commit
272346d8ad
3 files changed
+177
-1
No files matched your search
@@ -51,6 +51,15 @@ CREATE TABLE IF NOT EXISTS links(
|
||||
PRIMARY KEY(src, dst, rel));
|
||||
CREATE INDEX IF NOT EXISTS links_dst ON links(dst);
|
||||
|
||||
-- Every title a note has ever carried, so a rename does not silently break the
|
||||
-- inbound links that still spell the old one (§5). Resolution consults this only
|
||||
-- after the live title and slug miss, so a new note claiming a freed title wins.
|
||||
CREATE TABLE IF NOT EXISTS note_titles(
|
||||
note_id TEXT NOT NULL, title TEXT NOT NULL, slug TEXT NOT NULL, seen TEXT NOT NULL,
|
||||
PRIMARY KEY(note_id, title));
|
||||
CREATE INDEX IF NOT EXISTS note_titles_title ON note_titles(title);
|
||||
CREATE INDEX IF NOT EXISTS note_titles_slug ON note_titles(slug);
|
||||
|
||||
CREATE TABLE IF NOT EXISTS flags(
|
||||
id INTEGER PRIMARY KEY, note_id TEXT NOT NULL, reason TEXT, source TEXT,
|
||||
created TEXT, resolved INTEGER NOT NULL DEFAULT 0);
|
||||
@@ -140,6 +149,7 @@ class Database:
|
||||
chunk_columns = {r[1] for r in self._write.execute("PRAGMA table_info(chunks)")}
|
||||
if "tri_pending" not in chunk_columns:
|
||||
self._write.execute("ALTER TABLE chunks ADD COLUMN tri_pending INTEGER NOT NULL DEFAULT 0")
|
||||
self._backfill_note_titles()
|
||||
self._ensure_trigram_table()
|
||||
self._write.commit()
|
||||
rebuilt = self._ensure_vec_table()
|
||||
@@ -149,6 +159,29 @@ class Database:
|
||||
self._write.commit()
|
||||
return rebuilt
|
||||
|
||||
def _backfill_note_titles(self) -> None:
|
||||
"""Seed the alias table from the titles already in the index.
|
||||
|
||||
Without this, an upgraded index only learns a note's title the next time
|
||||
that note is reindexed — and a note that never changes is never
|
||||
reindexed, so the *first* rename after the upgrade would still break its
|
||||
inbound links, which is exactly the case the table exists to prevent.
|
||||
"""
|
||||
from mdmem.ids import slugify
|
||||
|
||||
assert self._write is not None
|
||||
rows = self._write.execute(
|
||||
"SELECT n.id, n.title FROM notes n "
|
||||
"LEFT JOIN note_titles t ON t.note_id = n.id AND t.title = n.title "
|
||||
"WHERE n.title IS NOT NULL AND n.title <> '' AND t.note_id IS NULL"
|
||||
).fetchall()
|
||||
for row in rows:
|
||||
self._write.execute(
|
||||
"INSERT OR IGNORE INTO note_titles(note_id,title,slug,seen) "
|
||||
"VALUES(?,?,?,strftime('%Y-%m-%dT%H:%M:%SZ','now'))",
|
||||
(row["id"], row["title"], slugify(row["title"])),
|
||||
)
|
||||
|
||||
def _ensure_trigram_table(self) -> None:
|
||||
"""Create/drop `chunks_tri` to match `self.trigram`.
|
||||
|
||||
|
||||
+35
-1
@@ -128,6 +128,7 @@ class Indexer:
|
||||
|
||||
self._write_links(w, note)
|
||||
self._mirror_flags(w, note)
|
||||
self._record_title(w, note)
|
||||
self._resolve_links_for(note)
|
||||
return pending
|
||||
|
||||
@@ -178,6 +179,7 @@ class Indexer:
|
||||
w.execute("DELETE FROM links WHERE src=?", (note_id,))
|
||||
w.execute("DELETE FROM links WHERE dst=?", (note_id,))
|
||||
w.execute("DELETE FROM flags WHERE note_id=?", (note_id,))
|
||||
w.execute("DELETE FROM note_titles WHERE note_id=?", (note_id,))
|
||||
w.execute("DELETE FROM notes WHERE id=?", (note_id,))
|
||||
|
||||
def inbound_sources(self, note_id: str) -> list[str]:
|
||||
@@ -210,9 +212,32 @@ class Indexer:
|
||||
seen.add(key)
|
||||
w.execute("INSERT OR REPLACE INTO links(src,dst,rel,origin) VALUES(?,?,?,?)", r)
|
||||
|
||||
@staticmethod
|
||||
def _record_title(w, note: Note) -> None:
|
||||
"""Remember this title for the note, so a later rename does not orphan
|
||||
the inbound links that still spell it (§5)."""
|
||||
if not note.title:
|
||||
return
|
||||
w.execute(
|
||||
"INSERT INTO note_titles(note_id,title,slug,seen) VALUES(?,?,?,?) "
|
||||
"ON CONFLICT(note_id, title) DO UPDATE SET seen=excluded.seen",
|
||||
(note.id, note.title, slugify(note.title), utcnow()),
|
||||
)
|
||||
|
||||
@staticmethod
|
||||
def _resolve(w, target: str) -> str:
|
||||
"""ULID, then exact title, then slug; otherwise 'unresolved:<text>' (§5)."""
|
||||
"""ULID, then exact title, then slug, then a former title; otherwise
|
||||
'unresolved:<text>' (§5).
|
||||
|
||||
The former-title lookup runs last on purpose. Resolution re-reads the
|
||||
link text on every reindex, so renaming a note used to turn every
|
||||
inbound `[[Old Title]]` into an unresolved link the next time its source
|
||||
was indexed — silently, and long after the rename. Consulting past
|
||||
titles fixes that without rewriting anyone else's body. Keeping it below
|
||||
the live lookups means a *new* note that claims the freed title still
|
||||
wins, which is the behaviour you want: the link text names whatever
|
||||
holds that title now.
|
||||
"""
|
||||
target = target.strip()
|
||||
if not target:
|
||||
return f"{UNRESOLVED}"
|
||||
@@ -224,6 +249,15 @@ class Indexer:
|
||||
return row[0]
|
||||
slug = slugify(target)
|
||||
row = w.execute("SELECT id, title FROM notes WHERE path LIKE ? LIMIT 1", (f"%/{slug}--%",)).fetchone()
|
||||
if row:
|
||||
return row[0]
|
||||
# A title may have been held by several notes over time; the most recent
|
||||
# holder is the least surprising answer, and it is deterministic.
|
||||
row = w.execute(
|
||||
"SELECT t.note_id FROM note_titles t JOIN notes n ON n.id = t.note_id "
|
||||
"WHERE t.title=? OR t.slug=? ORDER BY t.seen DESC, t.note_id DESC LIMIT 1",
|
||||
(target, slug),
|
||||
).fetchone()
|
||||
if row:
|
||||
return row[0]
|
||||
return f"{UNRESOLVED}{target}"
|
||||
|
||||
@@ -387,3 +387,112 @@ def test_store_vectors_requires_chunk_hash_match(idx):
|
||||
assert stored == 0
|
||||
assert db.read().execute("SELECT count(*) FROM chunks_vec").fetchone()[0] == 0
|
||||
assert db.read().execute("SELECT vec_pending FROM chunks").fetchone()[0] == 1
|
||||
|
||||
|
||||
# -- renames must not orphan the links that still spell the old title --------
|
||||
def _link_dst(db, src_id):
|
||||
row = db.read().execute("SELECT dst FROM links WHERE src=? AND rel='links'", (src_id,)).fetchone()
|
||||
return row["dst"] if row else None
|
||||
|
||||
|
||||
def test_renaming_a_note_keeps_inbound_links_resolved(idx):
|
||||
"""Resolution re-reads the link text on every reindex, so before the alias
|
||||
table a rename turned every inbound `[[Old Title]]` into an unresolved link
|
||||
the next time its source happened to be indexed — silently, and long after
|
||||
the rename itself."""
|
||||
indexer, vault, db = idx
|
||||
target = Note.new(title="Old Title", body="the target", type="fact")
|
||||
vault.write(target)
|
||||
indexer.index_note(vault.read(vault.find(target.id)))
|
||||
|
||||
source = Note.new(title="Source", body="see [[Old Title]] for detail", type="note")
|
||||
vault.write(source)
|
||||
indexer.index_note(vault.read(vault.find(source.id)))
|
||||
assert _link_dst(db, source.id) == target.id
|
||||
|
||||
renamed = vault.read(vault.find(target.id))
|
||||
renamed.set("title", "New Title")
|
||||
vault.write(renamed)
|
||||
indexer.index_note(vault.read(vault.find(target.id)))
|
||||
|
||||
# the inbound source is reindexed for some unrelated reason later on
|
||||
indexer.index_note(vault.read(vault.find(source.id)))
|
||||
assert _link_dst(db, source.id) == target.id, "the rename orphaned the inbound link"
|
||||
|
||||
# and a full rebuild does not detonate them all at once either
|
||||
indexer.scan(full=True)
|
||||
assert _link_dst(db, source.id) == target.id
|
||||
|
||||
|
||||
def test_a_new_note_claiming_a_freed_title_beats_the_alias(idx):
|
||||
"""The alias lookup runs last: link text names whatever holds that title
|
||||
now, not whatever held it first."""
|
||||
indexer, vault, db = idx
|
||||
first = Note.new(title="Shared Title", body="the original", type="fact")
|
||||
vault.write(first)
|
||||
indexer.index_note(vault.read(vault.find(first.id)))
|
||||
|
||||
renamed = vault.read(vault.find(first.id))
|
||||
renamed.set("title", "Renamed Away")
|
||||
vault.write(renamed)
|
||||
indexer.index_note(vault.read(vault.find(first.id)))
|
||||
|
||||
second = Note.new(title="Shared Title", body="the newcomer", type="fact")
|
||||
vault.write(second)
|
||||
indexer.index_note(vault.read(vault.find(second.id)))
|
||||
|
||||
source = Note.new(title="Source", body="points at [[Shared Title]]", type="note")
|
||||
vault.write(source)
|
||||
indexer.index_note(vault.read(vault.find(source.id)))
|
||||
assert _link_dst(db, source.id) == second.id
|
||||
|
||||
|
||||
def test_deleting_a_note_drops_its_former_titles(idx):
|
||||
"""Otherwise a dead ULID keeps winning resolution for a title nothing holds."""
|
||||
indexer, vault, db = idx
|
||||
target = Note.new(title="Doomed", body="x", type="fact")
|
||||
vault.write(target)
|
||||
indexer.index_note(vault.read(vault.find(target.id)))
|
||||
indexer.remove(target.id)
|
||||
assert db.read().execute(
|
||||
"SELECT count(*) FROM note_titles WHERE note_id=?", (target.id,)
|
||||
).fetchone()[0] == 0
|
||||
|
||||
source = Note.new(title="Source", body="points at [[Doomed]]", type="note")
|
||||
vault.write(source)
|
||||
indexer.index_note(vault.read(vault.find(source.id)))
|
||||
assert _link_dst(db, source.id) == "unresolved:Doomed"
|
||||
|
||||
|
||||
def test_existing_indexes_are_backfilled_so_the_first_rename_is_safe(tmp_path):
|
||||
"""An upgraded index only learns a title when the note is next reindexed, and
|
||||
an unchanged note never is — so without a backfill the first rename after the
|
||||
upgrade would still break its inbound links."""
|
||||
vault = Vault(tmp_path / "vault", tmp_path / ".history")
|
||||
vault.ensure_dirs()
|
||||
db = Database(tmp_path / "index.db", dim=8, trigram=True)
|
||||
db.open()
|
||||
indexer = Indexer(db, vault)
|
||||
target = Note.new(title="Before Upgrade", body="x", type="fact")
|
||||
vault.write(target)
|
||||
indexer.index_note(vault.read(vault.find(target.id)))
|
||||
source = Note.new(title="Source", body="see [[Before Upgrade]]", type="note")
|
||||
vault.write(source)
|
||||
indexer.index_note(vault.read(vault.find(source.id)))
|
||||
# simulate an index that predates the alias table
|
||||
with db.write() as w:
|
||||
w.execute("DELETE FROM note_titles")
|
||||
db.close()
|
||||
|
||||
db2 = Database(tmp_path / "index.db", dim=8, trigram=True)
|
||||
db2.open() # the migration backfills from the notes table
|
||||
try:
|
||||
indexer2 = Indexer(db2, vault)
|
||||
renamed = vault.read(vault.find(target.id))
|
||||
renamed.set("title", "After Upgrade")
|
||||
vault.write(renamed)
|
||||
indexer2.index_note(vault.read(vault.find(target.id)))
|
||||
indexer2.index_note(vault.read(vault.find(source.id)))
|
||||
assert _link_dst(db2, source.id) == target.id
|
||||
finally:
|
||||
db2.close()
|
||||
Reference in new issue
Block a user