From 8db9f3685bd0a481943ddbf4155bf50a0802fc1c Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Tue, 6 Oct 2026 21:31:41 -0400 Subject: [PATCH] server: one save path for a note's text, so a restored link gets its preview A body edit is a sequence: keep a revision, rename the note, lift #tags, commit, queue link previews. It was written out in PATCH, the item routes, restore and sync push, and the copies had drifted. Now they all call `notes/body.py: write_body`, which says how the old text is kept ("session", "always" for restore, "never" for a new note) and returns whether the text changed. The routes commit through `_commit_note`, which queues previews after the commit. Fixes, both red on 1a2f71e (run 8441): - restoring a revision queues previews for its links (#5164, audit B5); - a pushed note keeps the client's edit time when a standalone #tag is lifted. The lift's extra flush used to let `onupdate` stamp the server clock over it. Sync push also queues its previews after the batch commits, not mid-batch, where a fast fetch could look for a note that wasn't committed yet. Co-Authored-By: Claude Opus 5.5 --- src/inkwell/notes/__init__.py | 90 +++++++++++++---------------------- src/inkwell/notes/body.py | 46 ++++++++++++++++++ src/inkwell/sync.py | 52 ++++++++++---------- 3 files changed, 105 insertions(+), 83 deletions(-) create mode 100644 src/inkwell/notes/body.py diff --git a/src/inkwell/notes/__init__.py b/src/inkwell/notes/__init__.py index 5488dfa..87d0b2a 100644 --- a/src/inkwell/notes/__init__.py +++ b/src/inkwell/notes/__init__.py @@ -33,7 +33,6 @@ from ..models.note import Note from ..models.note_attachment import NoteAttachment from ..models.note_link_preview import NoteLinkPreview from ..models.note_revision import NoteRevision -from ..revisions import should_snapshot from .checklist import ( append_item, parse_items, @@ -47,6 +46,7 @@ from ..settings import get_setting from ..unfurl_queue import schedule as schedule_unfurls from ..unfurl import UnfurlError, unfurl from ._bp import bp +from .body import write_body from .helpers import ( ALLOWED_IMAGE_MIMES, VALID_FILTERS, @@ -402,22 +402,11 @@ async def create_note(): # are folded into the body, which is where a checklist lives now (M304). for text in item_texts: body = append_item(body, text) - note = Note( - owner_id=g.user_id, - display_title=derive_display_title(body), - body=body, - position=int(max_pos) + 1, - ) + note = Note(owner_id=g.user_id, body="", display_title="", position=int(max_pos) + 1) db.add(note) - await db.flush() # assign note.id before writing links # The FOLDED body: an item can carry a #tag too. - await _lift_and_reconcile_tags(db, note) - await db.commit() - await db.refresh(note) - # After the commit, never before it: the note is saved and the response is - # about to go out. Any link previews arrive on a later read. - schedule_unfurls(note.id, note.body) - return jsonify(await _serialize_note(db, note)), 201 + changed = await write_body(db, note, body, snapshot="never") + return await _commit_note(db, note, changed), 201 @bp.get("/") @@ -443,9 +432,12 @@ async def update_note(note_id: str): note = await _get_owned(db, note_id) if note is None: return not_found() - old_body = note.body - if "body" in data and isinstance(data["body"], str): - note.body = data["body"] + changed = False + if isinstance(data.get("body"), str): + # Version history: the first write of an editing session snapshots, not + # every write — see revisions.should_snapshot. Writing often is what lets + # a client autosave instead of hoarding text until it closes. + changed = await write_body(db, note, data["body"]) if "pinned" in data: note.pinned = bool(data["pinned"]) if "archived" in data: @@ -462,19 +454,21 @@ async def update_note(note_id: str): note.remind_at = remind_dt if "recurrence" in data: note.recurrence = normalize_recurrence(data["recurrence"]) - if "body" in data: - note.display_title = derive_display_title(note.body) - await _lift_and_reconcile_tags(db, note) - # Version history: snapshot the PRE-edit body, once per editing session - # rather than once per write — see revisions.should_snapshot. Writing often - # is what lets a client autosave instead of hoarding text until it closes. - if await should_snapshot(db, note.id, old_body, note.body): - db.add(NoteRevision(note_id=note.id, body=old_body)) - await db.commit() - await db.refresh(note) - if note.body != old_body: - schedule_unfurls(note.id, note.body) - return jsonify(await _serialize_note(db, note)) + return await _commit_note(db, note, changed) + + +async def _commit_note(db, note: Note, text_changed: bool): + """Commit a note write and answer with the note. + + Link previews are queued here, AFTER the commit and never before it: the fetch + reads the note in a session of its own, so a note it cannot see yet gets no + preview. `text_changed` is what `write_body` returned. + """ + await db.commit() + await db.refresh(note) + if text_changed: + schedule_unfurls(note.id, note.body) + return jsonify(await _serialize_note(db, note)) def _serialize_revision(rev: NoteRevision) -> dict: @@ -518,15 +512,11 @@ async def restore_revision(note_id: str, rev_id: str): return not_found() if note.body == rev.body: return jsonify(await _serialize_note(db, note)) # already at this version — no-op - # Snapshot the CURRENT state first, so restoring is itself undoable, then apply - # the revision — with the same body ripple as a normal edit. - db.add(NoteRevision(note_id=note.id, body=note.body)) - note.body = rev.body - note.display_title = derive_display_title(note.body) - await _lift_and_reconcile_tags(db, note) - await db.commit() - await db.refresh(note) - return jsonify(await _serialize_note(db, note)) + # Snapshot the CURRENT text unconditionally, so restoring is itself undoable, + # then apply the revision through the same path as any edit — which is what + # gives a restored link its preview back. + changed = await write_body(db, note, rev.body, snapshot="always") + return await _commit_note(db, note, changed) @bp.put("//labels") @@ -565,24 +555,8 @@ def _item_index(item_id: str) -> int | None: async def _rewrite_body(db, note: Note, body: str): - """Every item mutation is a body edit, so all of them land here. - - One place means one place that snapshots a revision, re-derives `#tags`, recomputes - the name and queues link unfurls — rather than three routes each remembering to. - Deliberately the same sequence the PATCH route runs for a body change, because it - IS a body change. - """ - old_body = note.body - if await should_snapshot(db, note.id, old_body, body): - db.add(NoteRevision(note_id=note.id, body=old_body)) - note.body = body - note.display_title = derive_display_title(body) - await _lift_and_reconcile_tags(db, note) - await db.commit() - await db.refresh(note) - if note.body != old_body: - schedule_unfurls(note.id, note.body) - return jsonify(await _serialize_note(db, note)) + """Every item mutation is a body edit, so it is saved exactly like one.""" + return await _commit_note(db, note, await write_body(db, note, body)) @bp.post("//items") diff --git a/src/inkwell/notes/body.py b/src/inkwell/notes/body.py new file mode 100644 index 0000000..64c16b2 --- /dev/null +++ b/src/inkwell/notes/body.py @@ -0,0 +1,46 @@ +"""Writing a note's text — the one path every surface goes through. + +A body edit is never only the body. It may keep a revision of the old text, it +renames the note, it lifts and reconciles `#tags`, and once committed it queues link +previews. That sequence used to be written out at each place a body could change — +PATCH, the item routes, restoring a revision and sync push — and the copies drifted: +restoring a revision never queued previews, so a restored link stayed a bare URL +(#5164). Now a caller states how the old text should be kept and this does the rest. + +Previews are the one part that cannot happen here. They are queued AFTER the commit +(`unfurl_queue.schedule`), because the fetch reads the note in a session of its own +and a note it cannot see yet gets no preview. So this returns whether the text +changed, and the caller queues them once its transaction is committed. +""" +from __future__ import annotations + +from typing import Literal + +from ..models.note import Note +from ..models.note_revision import NoteRevision +from ..revisions import should_snapshot +from .helpers import derive_display_title +from .tags import _lift_and_reconcile_tags + +# How the text being replaced is kept: +# "session" the first write of an editing session snapshots (revisions.should_snapshot) +# "always" snapshot unconditionally — restoring a revision must itself be undoable +# "never" a note being created has no earlier text to keep +Snapshot = Literal["session", "always", "never"] + + +async def write_body(db, note: Note, body: str, *, snapshot: Snapshot = "session") -> bool: + """Replace `note`'s text and everything derived from it. Returns whether the text + changed, which is what decides whether the caller queues previews after commit. + + Compared against the text AFTER tags are lifted, not the text passed in: a body + that only gained a standalone `#tag` ends up as it was, with a label attached. + """ + old_body = note.body or "" + if snapshot == "always" or (snapshot == "session" and await should_snapshot(db, note.id, old_body, body)): + db.add(NoteRevision(note_id=note.id, body=old_body)) + note.body = body + note.display_title = derive_display_title(body) + await db.flush() # a new note needs its id before its tag labels can point at it + await _lift_and_reconcile_tags(db, note) + return note.body != old_body diff --git a/src/inkwell/sync.py b/src/inkwell/sync.py index 6eed560..67f1d14 100644 --- a/src/inkwell/sync.py +++ b/src/inkwell/sync.py @@ -24,15 +24,12 @@ from .db import session_scope from .labeling import reconcile_manual_labels, resolve_owned_label_ids from .models.label import Label, NoteLabel from .models.note import Note -from .models.note_revision import NoteRevision -from .revisions import should_snapshot from .notes import ( - _lift_and_reconcile_tags, _serialize_notes, - derive_display_title, normalize_color, normalize_recurrence, ) +from .notes.body import write_body from .retention import purge_note from .serialize import serialize_label_sync from .unfurl_queue import schedule as schedule_unfurls @@ -205,8 +202,8 @@ def client_wins(client_edited_at: datetime | None, server_edited_at: datetime | def _assign_note_fields(note: Note, ch: dict) -> None: """Overwrite a note's scalar fields from a client's FULL-state change (sync is - whole-note, not a partial patch — the client sends its authoritative version).""" - note.body = ch["body"] if isinstance(ch.get("body"), str) else "" + whole-note, not a partial patch — the client sends its authoritative version). + The body is not one of them: it goes through `write_body`, like every edit.""" note.pinned = bool(ch.get("pinned")) note.archived = bool(ch.get("archived")) if ch.get("trashed"): @@ -244,7 +241,9 @@ async def _apply_note_manual_labels(db, note: Note, ch: dict) -> None: await reconcile_manual_labels(db, note, owned) -async def _apply_note(db, ch: dict) -> dict: +async def _apply_note(db, ch: dict, previews: list[tuple[uuid.UUID, str]]) -> dict: + """Apply one pushed note. A note whose text changed is appended to `previews` as + (id, final body), for the caller to queue link previews once the batch commits.""" raw_id = ch.get("id") try: nid = uuid.UUID(str(raw_id)) @@ -283,29 +282,25 @@ async def _apply_note(db, ch: dict) -> dict: elif note.purged_at is not None: note.purged_at = None # client re-created/edited → clear the tombstone - old_body = note.body _assign_note_fields(note, ch) - note.display_title = derive_display_title(note.body) + # Non-destructive LWW: the overwritten server body snapshots into history, subject + # to the same session window as a direct edit (revisions.should_snapshot). This + # path is why the window is a time rule rather than a flag on the wire: a client + # autosaving every second pushes a body change every second, and without the + # check the SERVER would snapshot each one no matter how restrained the client's + # own store was being. + body = ch["body"] if isinstance(ch.get("body"), str) else "" + if await write_body(db, note, body, snapshot="never" if creating else "session"): + # A note pushed from a linked client gets the same link previews as one typed + # into the web app; the client picks them up on its next pull. + previews.append((note.id, note.body)) + # AFTER write_body, whose flush lets the column's onupdate stamp the server's + # clock. Set here, the client's edit time is what the next flush writes. if edited_at is not None: note.updated_at = edited_at - # Non-destructive LWW: snapshot the overwritten server body into history — - # subject to the same session window as a direct edit (revisions.should_snapshot). - # This path is why the window is a time rule rather than a flag on the wire: a - # client autosaving every second pushes a body change every second, and without - # the check the SERVER would snapshot each one no matter how restrained the - # client's own store was being. - if not creating and await should_snapshot(db, note.id, old_body, note.body): - db.add(NoteRevision(note_id=note.id, body=old_body)) - await db.flush() # assign note.id before items/labels/links - await _lift_and_reconcile_tags(db, note) await _apply_note_manual_labels(db, note, ch) await db.flush() await db.refresh(note, ["sync_revision"]) - # A note pushed from a linked client gets the same link previews as one typed into - # the web app — the client picks them up on its next pull. Scheduled rather than - # awaited: a push batch must not wait on somebody else's website. - if creating or note.body != old_body: - schedule_unfurls(note.id, note.body) return { "id": str(nid), "entity": "note", @@ -391,6 +386,7 @@ async def push(): return jsonify({"error": f"too many changes in one push (max {MAX_PUSH})"}), 400 results = [] + previews: list[tuple[uuid.UUID, str]] = [] async with session_scope() as db: for ch in changes: if not isinstance(ch, dict): @@ -398,10 +394,16 @@ async def push(): continue entity = ch.get("entity") if entity == "note": - results.append(await _apply_note(db, ch)) + results.append(await _apply_note(db, ch, previews)) elif entity == "label": results.append(await _apply_label(db, ch)) else: results.append({"id": ch.get("id"), "status": "rejected", "error": "unknown entity"}) await db.commit() + # After the batch commits, never inside it: the fetch reads each note in a + # session of its own, and these were scheduled mid-batch until #5164, where a + # fast fetch could look for a note that was not committed yet. Scheduled rather + # than awaited — a push must not wait on somebody else's website. + for note_id, text in previews: + schedule_unfurls(note_id, text) return jsonify({"results": results})