fix(core): unresolve inbound relations on entity delete - #1358
Merged
Conversation
Deleting a note erased every link that pointed at it. The notes doing the pointing were never touched -- their markdown still said `[[That Note]]` -- but as far as the database was concerned no such link had ever been written. Ask for the links that lead nowhere and you got a clean report over a vault full of them. The row came back only if something re-indexed the source note, which nothing schedules. `relation.to_id` carried ON DELETE CASCADE, and `Entity.incoming_relations` carried a matching ORM delete-orphan cascade -- two mechanisms doing the same damage on two different routes. The watcher and directory runners take a bulk SQL DELETE, so the constraint ate the rows; the API single-entity delete takes `session.delete(entity)`, so the ORM did. to_id becomes ON DELETE SET NULL and incoming_relations trades delete-orphan for passive_deletes=True, handing the job to the database. from_id keeps CASCADE: an entity does own the relations it declares. Deleting a target now leaves its inbound rows in the unresolved state the indexer already produces for a link to a note that does not exist yet -- to_id null, to_name holding the source's link text -- so forward-reference resolution re-links them for free if the target is recreated. Nothing downstream needed teaching. Migration bcdbd5a942ca covers both backends. SQLite goes through batch_alter_table with a naming convention, because the initial-schema foreign key is unnamed and SQLite cannot address an anonymous constraint; Postgres drops and recreates relation_to_id_fkey. Rows already lost to the old behavior stay lost -- re-index the affected notes to recover them as unresolved. Tests cover all three delete paths plus the migration itself, which the rest of the SQLite suite cannot: it builds its schema from SQLAlchemy metadata, so a correct model with a missing migration would pass everywhere while every database on disk kept the old CASCADE. Two existing assertions in the watcher-parity and project-index tests expected `outgoing_relations == []` after a target delete. Both tests are about search-row repair; that line rode along as scenery and now asserts the surviving unresolved row instead. Fixes #1344 Signed-off-by: samwachtel <sammy@wachtel.us>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Member
Author
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
This was referenced Aug 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #1352 by @sammywachtel — their commit is cherry-picked here unchanged, authorship and sign-off preserved, so the full CI matrix (Postgres included) runs; fork PRs only get DCO/CLA/CodeQL. Will be rebase-merged so the commit lands on
mainas-is.Fixes #1344.
Summary
Deleting an entity took every inbound relation row with it —
relation.to_idcarriedON DELETE CASCADEandEntity.incoming_relationsa matching ORMdelete-orphan, one per delete route (bulk-SQL runners vs.session.delete). The source notes still contained the[[wikilink]], so a broken-link report came back clean over a vault full of them, and nothing re-created the rows short of re-indexing the source.to_idbecomesON DELETE SET NULLandincoming_relationsusespassive_deletes=True;from_idkeepsCASCADE. Inbound rows land in the unresolved state the indexer already produces (to_idNULL,to_namepreserved), and forward-reference resolution re-links them if the target is recreated — matchingdocs/DOMAIN_MODEL.md's "resolution enriches the statement; it does not replace it". Migrationbcdbd5a942ca(SQLite batch mode with a naming convention for the anonymous initial-schema FK; Postgres drop/recreaterelation_to_id_fkey), with migration tests that run the real Alembic chain.Verification (local, this branch, SQLite)
ruff check/ruff format --check/ty check src tests test-int(all extras) — cleantests/api tests/index tests/indexing tests/repository tests/services tests/db— 2186 passedtest-int(not slow/benchmark) — 482 passedThe contributor also verified the migration against real Postgres (
pg_constraint.confdeltype=nforto_id,cforfrom_id) and ranjust testacross both backends; see #1352 for the red/green matrix showing the DB and ORM halves are independently load-bearing.Out of scope, tracked separately: the stale
search_indexrow on the API delete path (#1351).🤖 Generated with Claude Code
https://claude.ai/code/session_017STCpbNsYjZgUdftxgEAZ4