diff --git a/core/src/local/store.rs b/core/src/local/store.rs index e267184..9c98e56 100644 --- a/core/src/local/store.rs +++ b/core/src/local/store.rs @@ -375,6 +375,43 @@ pub fn create_note(conn: &Connection, input: &NoteCreateInput) -> rusqlite::Resu load_note(conn, &id) } +/// How long one editing session is assumed to last. +/// +/// Inside this window a note's body may be written any number of times and only the +/// FIRST write snapshots. That is what makes an idle-debounced autosave affordable: +/// a write costs a write, not a write plus a revision. +const REVISION_WINDOW_MINUTES: i64 = 10; + +/// Whether a body change earns a snapshot of the pre-edit body. +/// +/// Two conditions. The body must actually differ — re-saving identical text is not a +/// version of anything. And the note must not already carry a revision from this +/// editing session. +/// +/// The session rule is what keeps version history worth reading. Because +/// [`snapshot_revision`] stores the body as it was BEFORE the edit, the first write +/// of a session captures the note as you found it, and every write after it inside +/// the window adds nothing. One revision per sitting falls out of the window on its +/// own — no "commit" the client has to declare, and no protocol surface to carry it, +/// which matters because sync-apply takes this same path. +fn should_snapshot(conn: &Connection, id: &str, new_body: &str) -> rusqlite::Result { + let current: String = + conn.query_row("SELECT body FROM notes WHERE id = ?1", [id], |r| r.get(0))?; + if current == new_body { + return Ok(false); + } + // String comparison, not date maths: timestamps are RFC3339 UTC with a fixed + // millisecond field (see the module header), so lexical order IS chronological. + let cutoff = (Utc::now() - Duration::minutes(REVISION_WINDOW_MINUTES)) + .to_rfc3339_opts(SecondsFormat::Millis, true); + let recent: i64 = conn.query_row( + "SELECT COUNT(*) FROM note_revisions WHERE note_id = ?1 AND created_at >= ?2", + params![id, cutoff], + |r| r.get(0), + )?; + Ok(recent == 0) +} + fn snapshot_revision(conn: &Connection, id: &str) -> rusqlite::Result<()> { let body: String = conn.query_row("SELECT body FROM notes WHERE id = ?1", [id], |r| r.get(0))?; @@ -391,9 +428,12 @@ pub fn update_note(conn: &Connection, id: &str, changes: &Value) -> rusqlite::Re .as_object() .ok_or_else(|| rusqlite::Error::InvalidParameterName("changes must be an object".into()))?; - // Snapshot the pre-edit body before changing it (version history). - if obj.contains_key("body") { - snapshot_revision(conn, id)?; + // Snapshot the pre-edit body before changing it (version history) — but only + // once per editing session, and only if it actually changed. See should_snapshot. + if let Some(body) = obj.get("body").and_then(|v| v.as_str()) { + if should_snapshot(conn, id, body)? { + snapshot_revision(conn, id)?; + } } for (k, v) in obj { diff --git a/src/thoughtsync/notes/__init__.py b/src/thoughtsync/notes/__init__.py index 82f8490..f8e07a3 100644 --- a/src/thoughtsync/notes/__init__.py +++ b/src/thoughtsync/notes/__init__.py @@ -34,6 +34,7 @@ from ..models.note_attachment import NoteAttachment from ..models.note_item import NoteItem from ..models.note_link_preview import NoteLinkPreview from ..models.note_revision import NoteRevision +from ..revisions import should_snapshot from ..responses import json_error, not_found, parse_uuid from ..retention import purge_note from ..settings import get_setting @@ -485,8 +486,10 @@ async def update_note(note_id: str): if "body" in data: note.display_title = await _name_for(db, note) await _reconcile_tags(db, note) - # Version history: snapshot the PRE-edit body whenever it changed. - if note.body != old_body: + # 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) diff --git a/src/thoughtsync/revisions.py b/src/thoughtsync/revisions.py new file mode 100644 index 0000000..2b1affb --- /dev/null +++ b/src/thoughtsync/revisions.py @@ -0,0 +1,55 @@ +"""When a body change is worth keeping a version of. + +A note's body used to snapshot into `note_revisions` on EVERY write, which made a +write expensive — and the clients compensated by writing as rarely as they could +get away with, saving only when an editor closed. That is durability paying for +version history: a crash mid-session lost everything typed, so that the revision +list would stay tidy. The safety property is worth more than the feature it was +subsidising. + +The rule here breaks that trade. A body change earns a snapshot only if it is the +first one of an editing session, so a client may write as often as it likes. + +Session granularity falls out of the window rather than being declared. A snapshot +stores the body as it was BEFORE the edit, so the first write of a sitting captures +the note as you found it and every write after it inside the window adds nothing — +one revision per sitting, with no "commit" flag for a client to send and no wire +surface to carry it. That last part is why this is a time rule and not a protocol +one: `sync.py` applies pushed bodies through the same check, so a client autosaving +every second cannot make the server snapshot every second either. + +Deliberately NOT applied to restoring a revision. That is a considered act rather +than a keystroke, and it snapshots unconditionally so restoring is itself undoable. +""" +from __future__ import annotations + +import uuid +from datetime import datetime, timedelta, timezone + +from sqlalchemy import select + +from .models.note_revision import NoteRevision + +# How long one editing session is assumed to last. A constant rather than a setting: +# it is not a preference anyone holds, and the value only has to be longer than a +# sitting and shorter than the gap between two of them. Promote it to the settings +# registry (rule 25) if that ever stops being true. +REVISION_WINDOW_MINUTES = 10 + + +async def should_snapshot(db, note_id: uuid.UUID, old_body: str, new_body: str) -> bool: + """Whether `old_body` should be kept as a revision before `new_body` replaces it. + + False when the text did not actually change — re-saving identical bytes is not a + version of anything — and False when this note already has a revision from the + current session. + """ + if old_body == new_body: + return False + cutoff = datetime.now(timezone.utc) - timedelta(minutes=REVISION_WINDOW_MINUTES) + recent = await db.scalar( + select(NoteRevision.id) + .where(NoteRevision.note_id == note_id, NoteRevision.created_at >= cutoff) + .limit(1) + ) + return recent is None diff --git a/src/thoughtsync/sync.py b/src/thoughtsync/sync.py index 775e2b3..ad46c6d 100644 --- a/src/thoughtsync/sync.py +++ b/src/thoughtsync/sync.py @@ -26,6 +26,7 @@ from .models.label import Label, NoteLabel from .models.note import Note from .models.note_item import NoteItem from .models.note_revision import NoteRevision +from .revisions import should_snapshot from .notes import ( _reconcile_tags, _serialize_notes, @@ -317,8 +318,13 @@ async def _apply_note(db, ch: dict) -> dict: note.display_title = derive_display_title(note.body, _first_item_text(ch)) if edited_at is not None: note.updated_at = edited_at - # Non-destructive LWW: snapshot the overwritten server body into history. - if not creating and note.body != old_body: + # 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 _apply_note_items(db, note, ch) diff --git a/tests/test_integration.py b/tests/test_integration.py index f99cf57..f94d0bf 100644 --- a/tests/test_integration.py +++ b/tests/test_integration.py @@ -16,6 +16,7 @@ deployment has ever seen. from __future__ import annotations import uuid +from datetime import datetime, timedelta, timezone import pytest import pytest_asyncio @@ -30,6 +31,8 @@ from thoughtsync.models.user import User from thoughtsync.settings import get_setting, live, refresh_live, reset_live, set_settings from thoughtsync.notes.helpers import derive_display_title from thoughtsync.models.note_link_preview import NoteLinkPreview +from thoughtsync.models.note_revision import NoteRevision +from thoughtsync.revisions import REVISION_WINDOW_MINUTES, should_snapshot from thoughtsync.sync import _apply_note_items from thoughtsync.unfurl_queue import _unfurl_new_urls, detect_urls @@ -414,3 +417,70 @@ async def test_the_security_group_reaches_the_admin_ui(app_client, db): assert row["type"] == "int" assert row["minimum"] is not None and row["maximum"] is not None assert row["description"], f"{row['key']} has no description to explain itself" + + +async def _revision_count(db, note_id) -> int: + rows = (await db.scalars(select(NoteRevision.id).where(NoteRevision.note_id == note_id))).all() + return len(rows) + + +async def test_a_session_of_edits_costs_one_revision(db, owner): + """The change that makes autosave affordable. + + Version history used to snapshot on EVERY body write, so the clients saved as + rarely as they could — only when an editor closed — and a crash mid-session lost + everything typed. Durability was paying for history. Now a sitting earns one + revision no matter how many times it is written, so a client can write whenever + it likes. + """ + note = Note(owner_id=owner.id, body="one", display_title="one") + db.add(note) + await db.commit() + + # A session's worth of autosaves. + for text_ in ("one two", "one two three", "one two three four"): + if await should_snapshot(db, note.id, note.body, text_): + db.add(NoteRevision(note_id=note.id, body=note.body)) + note.body = text_ + await db.commit() + + assert await _revision_count(db, note.id) == 1 + + # And it is the body as it was BEFORE the sitting, not some midpoint — which is + # what makes one-per-session the useful granularity rather than an arbitrary one. + kept = (await db.scalars(select(NoteRevision.body).where(NoteRevision.note_id == note.id))).all() + assert kept == ["one"] + + +async def test_rewriting_the_same_text_is_not_a_version(db, owner): + note = Note(owner_id=owner.id, body="unchanged", display_title="unchanged") + db.add(note) + await db.commit() + + assert await should_snapshot(db, note.id, note.body, "unchanged") is False + assert await _revision_count(db, note.id) == 0 + + +async def test_a_later_sitting_earns_its_own_revision(db, owner): + """The window has to REOPEN, or a note edited daily would keep only its first + version forever — which would be a worse history than the one we replaced.""" + note = Note(owner_id=owner.id, body="today", display_title="today") + db.add(note) + await db.flush() + # A revision from longer ago than one session: the clock is not mocked, the row + # is simply written with an older timestamp, which is what the query reads. + stale = datetime.now(timezone.utc) - timedelta(minutes=REVISION_WINDOW_MINUTES + 1) + db.add(NoteRevision(note_id=note.id, body="yesterday", created_at=stale)) + await db.commit() + + assert await should_snapshot(db, note.id, note.body, "tomorrow") is True + + +async def test_a_revision_inside_the_window_blocks_another(db, owner): + note = Note(owner_id=owner.id, body="draft", display_title="draft") + db.add(note) + await db.flush() + db.add(NoteRevision(note_id=note.id, body="earlier", created_at=datetime.now(timezone.utc))) + await db.commit() + + assert await should_snapshot(db, note.id, note.body, "draft revised") is False