Skip to content

fix: sync renames a note's row in place when its id changes - #159

Open
fduple wants to merge 2 commits into
jordan-gibbs:mainfrom
fduple:pr/sync-id-rewrite-in-place
Open

fduple wants to merge 2 commits into
jordan-gibbs:mainfrom
fduple:pr/sync-id-rewrite-in-place

Conversation

@fduple

@fduple fduple commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

What this changes

sync renames a note's row in place when its file's id: changes, children included; note rm deletes asset files by the assets rows, not by the directory named after the id.

Why

A rewritten id: went through the upsert as an INSERT of the new id; UNIQUE(notes.path) refused it, the stale row kept the old id, and the file that really owns that id collided with it on every sync. Renaming the file instead re-indexes it as a new note and drops its claims.

Reproduction on main in a fresh hpr init vault (timings dropped; the vault path in the last line shortened to <vault>):

$ hpr sync    # research/notes/topic-2.md carries id: topic
Synced: +1 ~0 -0 =0
$ hpr sync    # research/notes/topic.md added with id: topic
Synced: +0 ~0 -0 =1
  Error: research/notes/topic.md: id collision: 'topic' already belongs to 'research/notes/topic-2.md'. Skipped to avoid silent overwrite.
$ hpr sync    # only the id: line of topic-2.md rewritten, to topic-2
Synced: +0 ~0 -0 =0
  Error: research/notes/topic.md: id collision: 'topic' already belongs to 'research/notes/topic-2.md'. Skipped to avoid silent overwrite.
  Error: <vault>/research/notes/topic-2.md: UNIQUE constraint failed: notes.path

Every later sync repeats both errors. With this change the third sync prints Synced: +1 ~1 -0 =0, the fourth Synced: +0 ~0 -0 =2, and both files are indexed under their own ids.

How

When the row at a file's path carries a different id that no other path owns, _rename_note_id drops the rows the upsert rebuilds and re-points the rest (embeddings, claims, assets, sources, escalations) under PRAGMA defer_foreign_keys, as no foreign key declares ON UPDATE. The rename needs a declared id; updates run before adds so a freed id reaches its new owner in the same pass. A new file named after the old id keeps the rows fetch recorded for it (URL rows matching its source:, assets rows inside research/assets/<old_id>/) unless the renamed file itself has that stem; a new file named after the new id and claiming it beats the edit, and one named after either id whose frontmatter does not parse makes the rename wait. Per file, rename and upsert share a SAVEPOINT; a whole-transaction rollback (SQLITE_FULL) stops the pass with zero counts and a "nothing from this sync pass was kept" error.

note rm removed research/assets/<id>/ by name, after a rename another note's directory. It now removes the files its own rows name under research/assets/, plus what research/assets/<id>/ holds that no other note's row names, by file identity; nothing outside research/assets/, a symlinked research/assets/<id> is not walked, and a file it cannot remove is reported under assets_not_removed.

How it was verified

  • tests/test_core/test_sync.py (16 new tests, 20 cases) and tests/test_cli/test_note_rm_assets.py (7); on main 26 cases fail on an assertion and one cannot run (it imports the new table constants).
  • pytest tests/: 1746 passed on 3.11, 3.12, 3.13 and 3.14 (Linux); ruff check src/ tests/ clean. Windows and macOS were not run.
  • With fix: sync indexes the final report, which has no frontmatter by design #154 (sync indexes the header-less final report) this also clears the limit named there, a report first indexed with a leaked header failing every later sync once stripped; I ran the two together.

Limits

  • A pass that stops still exits 0 with "ok": true under --json, as on main for any per-file error.
  • A database at its size limit blocks every pass until fixed; on main later files were still indexed.
  • An id edit now beats a new file not named after the id, a moved note included; on main the new file won.
  • note mv still drops the moved note's claims and nulls its sources row's note_id, as on main.
  • Asset files do not move on a rename; rows keep their full paths.
  • removed_assets labels are now always vault-relative (main printed bare names in the note's own directory).
  • escalations.suggested_by and [[old-id]] links in other notes keep the old id.

Scope check — the review is much faster when the diff contains only the change being proposed:

  • No unrelated reformatting, whitespace, or prose rewriting outside the lines this change needs
  • CHANGELOG.md touched only to add an entry under [Unreleased]
  • No edits to user-facing output strings, docstrings, or test fixtures unrelated to this fix
  • If you ran a formatter or an automated cleanup pass, its results are in a separate PR

fduple and others added 2 commits October 1, 2026 21:08
A note file whose `id:` changed at a path that already has a row went
through the sync upsert as an INSERT of the new id. UNIQUE(notes.path)
refused it, the stale row kept the old id, and the file that really owns
that id collided with it on every sync. Renaming the file instead
re-indexed it as a new note and dropped its claims; its sources row kept
the URL with note_id NULL.

Reproduction on main, in a fresh `hpr init` vault (timings dropped; the
vault path in the last line shortened to <vault>):

  $ hpr sync    # research/notes/topic-2.md carries id: topic
  Synced: +1 ~0 -0 =0
  $ hpr sync    # research/notes/topic.md added with id: topic
  Synced: +0 ~0 -0 =1
    Error: research/notes/topic.md: id collision: 'topic' already belongs to 'research/notes/topic-2.md'. Skipped to avoid silent overwrite.
  $ hpr sync    # only the id: line of topic-2.md rewritten, to topic-2
  Synced: +0 ~0 -0 =0
    Error: research/notes/topic.md: id collision: 'topic' already belongs to 'research/notes/topic-2.md'. Skipped to avoid silent overwrite.
    Error: <vault>/research/notes/topic-2.md: UNIQUE constraint failed: notes.path

Every later sync repeats both errors and `notes` keeps the one row
('topic', 'research/notes/topic-2.md'). With this change the third sync
prints `Synced: +1 ~1 -0 =0`, the fourth `Synced: +0 ~0 -0 =2`, and
`notes` holds ('topic', 'research/notes/topic.md') and ('topic-2',
'research/notes/topic-2.md').

Fix: execute_sync keeps a path -> id map beside the id -> path map. When
the row at a file's path carries a different id that no other path owns,
_rename_note_id drops the rows the upsert rebuilds and re-points the
rest (embeddings, claims, assets, sources, escalations) under PRAGMA
defer_foreign_keys, as no foreign key declares ON UPDATE; a child table
it missed fails the commit. The rename needs an id the frontmatter
declares, so a file whose frontmatter stops parsing is reported, not
re-keyed to its file name. Updates run before adds, so a freed id
reaches the new file that owns it in the same pass.

Fetch records a note's URL and assets under the stem of the file it
wrote, so a topic.md fetched while topic-2.md held the id has rows
under 'topic' although sync refused it. Unless the renamed file's own
stem is the old id, such a file keeps the URL rows matching its
`source:` and the assets rows inside research/assets/<old_id>/; rows
kept for a claimant whose own upsert then fails are orphaned as a
delete would. A new file named after the new id and claiming it beats
the id edit; one named after either id whose frontmatter does not parse
makes the rename wait, with an error naming it.

Each file's rename and upsert run inside a SAVEPOINT, and the maps
change only after the release. That also fixes an ordinary edit whose
upsert fails part-way: on main the notes row had already taken the new
hash and mtime, so the next sync counted the file as unchanged and the
error was never shown again. When SQLite rolls back the whole
transaction (SQLITE_FULL does), the maps still describe the undone
deletes and renames and a later file could land on a restored row and
take its children, so the pass now stops with zero counts and the error
entry adds "; nothing from this sync pass was kept: fix that error and
run sync again"; on main the loop carried on in a new transaction,
committing the files after the failing one and counting the undone ones
before it. The tag_aliases read no longer swallows every exception.

note rm removed research/assets/<id>/ by name, which after a rename is
another note's directory. It now removes the files its own assets rows
name under research/assets/, plus what research/assets/<id>/ holds that
no other note's row names, by file identity; nothing outside
research/assets/, a symlinked research/assets/<id> is not walked and
stays, and a file it cannot remove is reported under assets_not_removed
while the delete finishes.

Tests: tests/test_core/test_sync.py (16 new tests, 20 cases) and
tests/test_cli/test_note_rm_assets.py (7). On main 26 cases fail on an
assertion and one cannot run
(test_sync_id_rewrite_covers_every_table_with_a_foreign_key_to_notes
imports the new table constants). The whole-rollback tests fill a
database capped with PRAGMA max_page_count; the delete-loop one stands
in with a conn.rollback() call. The symlink tests skip where a symlink
cannot be made; the unremovable-file test skips on Windows and as root.

`pytest tests/`: 1746 passed on 3.11, 3.12, 3.13 and 3.14 (Linux);
`ruff check src/ tests/` is clean. Windows and macOS were not run.

Limits:
- A pass that stops still exits 0, with "ok": true under --json and
  `+0 ~0 -0` in the summary, as on main for any per-file error; only
  the error entry says the pass was dropped.
- A file whose upsert rolls the whole transaction back on every run (a
  database at its size limit) blocks every pass until the cause is
  fixed; on main the files after it were still indexed.
- An id edit now beats a new file NOT named after the id that claims it
  in the same pass (on main the new file won); the loser is refused on
  every sync either way. A note moved to a new path counts as such a
  new file.
- note mv still goes through a delete and an add, so the moved note's
  claims go and its sources row keeps the URL with note_id NULL, as on
  main; not touched here.
- Asset files do not move on a rename; the rows keep their full paths.
  fetch still writes screenshot.png with no collision check, and its
  duplicate-URL race loser saves assets under the winner's id; both
  pre-exist and are left alone.
- removed_assets labels are now always vault-relative (main printed a
  bare name for a file in the note's own directory); nothing in the
  repo parses them.
- escalations.suggested_by and [[old-id]] links in other notes keep the
  old id.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A dangling symlink, or one pointing outside research/assets/, in
research/assets/<id>/ was skipped (silently, or reported as left in
place), so the directory was never emptied and stayed on disk; the old
shutil.rmtree removed both. A symlink is now unlinked as itself, which
never touches its target, when it sits below research/assets/ and no
other note's assets row names it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TZrbJKxzwsFkWndioJB7q6

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant