A write should not cost a revision
CI & Build / Build now, or wait for Android? (push) Successful in 3s
CI & Build / Python lint (push) Successful in 4s
CI & Build / TypeScript typecheck (push) Successful in 7s
CI & Build / Python tests (push) Successful in 12s
CI & Build / integration (push) Successful in 19s
CI & Build / Build & push image (push) Skipped
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 3m3s
Desktop (Tauri) / Tauri desktop (Linux) (push) Successful in 5m21s
Desktop (Tauri) / Update manifest (push) Successful in 3s
Android / Kotlin + Rust (APK) (push) Successful in 7m42s
CI & Build / Build now, or wait for Android? (push) Successful in 3s
CI & Build / Python lint (push) Successful in 4s
CI & Build / TypeScript typecheck (push) Successful in 7s
CI & Build / Python tests (push) Successful in 12s
CI & Build / integration (push) Successful in 19s
CI & Build / Build & push image (push) Skipped
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 3m3s
Desktop (Tauri) / Tauri desktop (Linux) (push) Successful in 5m21s
Desktop (Tauri) / Update manifest (push) Successful in 3s
Android / Kotlin + Rust (APK) (push) Successful in 7m42s
Every body change snapshotted into history — core/src/local/store.rs and notes/__init__.py both — so a write was expensive, and the clients compensated by writing as rarely as they could. BoardViewModel says it outright: "Saved on close rather than per keystroke, so a session of typing costs one write and one revision snapshot." That is durability paying for version history. An app kill 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, and no comparable product makes this trade: Keep and Apple Notes write continuously with no history, Docs and Notion write continuously and coalesce history behind the scenes, Obsidian debounces and snapshots on an interval. Save-on-close is the outlier, and this coupling is why we had it. A body change now earns a snapshot only if it is the first of an editing session — the body actually differs, and the note carries no revision from the last ten minutes. 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 is why it 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 — which a client-declared commit point could not have guaranteed without a protocol bump. Restoring a revision still snapshots unconditionally: a considered act, not a keystroke, and it stays undoable. Unblocks idle-debounced autosave, an honest updated_at, and the "Edited just now" line the editor is getting. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+42
-2
@@ -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<bool> {
|
||||
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,10 +428,13 @@ 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 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 {
|
||||
match k.as_str() {
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user