core: the body is the checklist, and checklist_items is gone
Desktop (Tauri) / Tauri desktop (Linux) (push) Failing after 2m30s
Desktop (Tauri) / Windows installer (cross-compiled) (push) Successful in 2m47s
Desktop (Tauri) / Update manifest (push) Skipped
Android / Kotlin + Rust (APK) (push) Successful in 6m45s

M304 steps 2 and the client half of 4, together — they cannot be separated. A
commit where the store writes items into the body while push.rs still reads them
from a table is one that silently pushes the wrong list, and dev publishes to the
dev channel on every green build.

Store:
  * load_items becomes items_of(body) — a parse, not a query. An item's id is its
    ORDINAL, which is all it ever amounted to: push.rs sent text and checked and
    never an id, and both sides replaced the whole list on every sync.
  * add_item / update_item / delete_item route through update_note, so they get
    revision snapshotting, #tag re-derivation and the dirty/updated_at bookkeeping
    without any of it being written a second time.
  * create_note folds its items: input into the body, and syncs tags from the
    FOLDED body — an item can carry a #tag too.
  * display_title no longer takes items, because items ARE body lines now. It
    strips the task marker instead: a list-only note is still named by its first
    item, and calling that note "- [ ] milk" would show someone the storage.

Wire: items leave it. A second copy of data already in the body field of the same
message is how the two come to disagree. CLIENT_PROTOCOL_VERSION and
MIN_SERVER_PROTOCOL_VERSION go to 3, which is what makes this safe to land before
the server: a v3 client refuses a v2 server outright rather than pushing a body
whose list the old _apply_note_items would then delete.

Schema v8 folds every existing row into its note's body before dropping the
table. Written in Rust, not SQL: the fold has to produce exactly what
derive::append_item produces, and group_concat only gained a guaranteed ORDER BY
in SQLite 3.44 — a checklist that quietly reordered itself during a migration
would be a poor way to learn that. updated_at and dirty are deliberately left
alone, because the server's migration folds the same rows the same way and both
sides land on identical bodies; marking every note dirty would push a body the
server already has, from every device at once.

NOT deployable yet. The server still speaks v2 and still has note_items, so a
client built from this will refuse to sync until the server half lands.
This commit is contained in:
2026-08-24 00:41:23 -04:00
parent d0e3e48943
commit 668f7faf03
8 changed files with 331 additions and 184 deletions
+29 -6
View File
@@ -155,6 +155,17 @@ fn render_task_line(indent: &str, bullet: char, checked: bool, text: &str) -> St
}
}
/// The text of a line with its task marker removed, or the line as it was.
///
/// For naming a note: a list-only note is named by its first item, and calling one
/// "- [ ] milk" would be showing someone the storage instead of the note.
pub fn strip_marker(line: &str) -> &str {
match parse_task_line(line) {
Some(t) => t.text,
None => line,
}
}
/// Every checklist item in `body`, in the order they appear.
pub fn extract_items(body: &str) -> Vec<DerivedItem> {
let mut out = Vec::new();
@@ -237,8 +248,12 @@ pub fn remove_item(body: &str, index: usize) -> String {
/// cosmetic: the server migration folds existing rows into bodies using the same
/// layout, so an export taken before the migration and one taken after have to
/// agree byte for byte.
pub fn append_item(body: &str, text: &str) -> String {
let line = render_task_line("", '-', false, text.trim());
///
/// `checked` is a parameter rather than always false because the two migrations that
/// fold existing rows into bodies have to carry the state those rows were in. A new
/// item from the UI passes false.
pub fn append_item(body: &str, text: &str, checked: bool) -> String {
let line = render_task_line("", '-', checked, text.trim());
let trimmed = body.trim_end_matches('\n');
if trimmed.trim().is_empty() {
return line;
@@ -378,13 +393,21 @@ mod tests {
// Prose then a blank line then the list — byte-for-byte what
// import_export.py:_note_markdown writes, which is what the server
// migration will fold existing rows into.
assert_eq!(append_item("a note", "milk"), "a note\n\n- [ ] milk");
assert_eq!(append_item("a note", "milk", false), "a note\n\n- [ ] milk");
// Nothing between consecutive items.
let one = "a note\n\n- [ ] milk";
assert_eq!(append_item(one, "eggs"), format!("{one}\n- [ ] eggs"));
assert_eq!(append_item(one, "eggs", false), format!("{one}\n- [ ] eggs"));
// A list-only note starts at the first line.
assert_eq!(append_item("", "milk"), "- [ ] milk");
assert_eq!(append_item("\n\n", "milk"), "- [ ] milk");
assert_eq!(append_item("", "milk", false), "- [ ] milk");
assert_eq!(append_item("\n\n", "milk", false), "- [ ] milk");
// Carries state, which is what the two migrations need of it.
assert_eq!(append_item("", "done", true), "- [x] done");
}
#[test]
fn strip_marker_names_a_list_only_note() {
assert_eq!(strip_marker("- [x] milk"), "milk");
assert_eq!(strip_marker("just prose"), "just prose");
}
#[test]
+192 -1
View File
@@ -6,7 +6,9 @@
//!
//! Migrations are gated on `PRAGMA user_version`; bump it and add a block per change.
use rusqlite::Connection;
use rusqlite::{params, Connection, OptionalExtension};
use crate::local::derive;
const SCHEMA_V1: &str = r#"
CREATE TABLE notes (
@@ -54,6 +56,8 @@ CREATE TABLE checklist_items (
position INTEGER NOT NULL DEFAULT 0
);
CREATE INDEX idx_items_note ON checklist_items (note_id);
-- Both dropped in v8; kept here so an existing database has something to migrate
-- FROM, exactly as `kind` above is kept for v6.
CREATE TABLE attachments (
id TEXT PRIMARY KEY,
@@ -180,6 +184,73 @@ ALTER TABLE notes DROP COLUMN title;
ALTER TABLE note_revisions DROP COLUMN title;
"#;
// v8 (M304): `checklist_items` is gone. The body IS the checklist — a `- [ ] milk`
// line is the item — so a list can sit between two paragraphs instead of only after
// them, which a side table could never express no matter how it was styled.
//
// Rust rather than a SQL const, for two reasons. The fold has to produce EXACTLY what
// `derive::append_item` produces, and expressing that in SQL would be a second
// implementation of the layout rule. And `group_concat` only gained a guaranteed
// ORDER BY in SQLite 3.44 — a checklist that silently reordered itself during the
// migration would be a poor way to find that out.
//
// `updated_at` and `dirty` are deliberately NOT touched. The server's Alembic
// migration folds the same rows with the same spacing, so both sides land on
// identical bodies and this needs no sync at all; marking every note dirty would
// push a body the server already has, and would do it for every device at once.
fn migrate_v8(conn: &Connection) -> rusqlite::Result<()> {
// Grouped in one pass — the query is ordered by note, so a change of note_id is
// the group boundary. `rowid` breaks ties, because `position` was only ever
// advisory and two rows sharing one is not a reason to reorder someone's list.
let mut grouped: Vec<(String, Vec<(String, bool)>)> = Vec::new();
{
let mut stmt = conn.prepare(
"SELECT note_id, text, checked FROM checklist_items
ORDER BY note_id ASC, position ASC, rowid ASC",
)?;
let mut rows = stmt.query([])?;
while let Some(row) = rows.next()? {
let note_id: String = row.get(0)?;
let text: String = row.get(1)?;
let checked: bool = row.get(2)?;
match grouped.last_mut() {
Some((id, items)) if *id == note_id => items.push((text, checked)),
_ => grouped.push((note_id, vec![(text, checked)])),
}
}
}
for (note_id, items) in grouped {
let existing: Option<String> = conn
.query_row(
"SELECT body FROM notes WHERE id = ?1",
[&note_id],
|r| r.get(0),
)
.optional()?;
// An item whose note is already gone has nothing to fold into. The foreign key
// should make this impossible; skipping costs nothing, and failing here would
// leave the only copy of someone's notes half-migrated.
let mut body = match existing {
Some(b) => b,
None => continue,
};
for (text, checked) in items {
body = derive::append_item(&body, &text, checked);
}
conn.execute(
"UPDATE notes SET body = ?1 WHERE id = ?2",
params![body, note_id],
)?;
}
conn.execute_batch(
"DROP INDEX IF EXISTS idx_items_note;
DROP TABLE checklist_items;",
)?;
Ok(())
}
/// Bring the database up to the latest schema. Idempotent.
pub fn migrate(conn: &Connection) -> rusqlite::Result<()> {
conn.execute_batch("PRAGMA foreign_keys = ON;")?;
@@ -212,5 +283,125 @@ pub fn migrate(conn: &Connection) -> rusqlite::Result<()> {
conn.execute_batch(SCHEMA_V7)?;
conn.execute_batch("PRAGMA user_version = 7;")?;
}
if version < 8 {
migrate_v8(conn)?;
conn.execute_batch("PRAGMA user_version = 8;")?;
}
Ok(())
}
#[cfg(test)]
mod tests {
use super::*;
/// A database as it stood before M304 — items still in their own table.
fn v7_db() -> Connection {
let conn = Connection::open_in_memory().expect("open");
conn.execute_batch("PRAGMA foreign_keys = ON;").expect("fk");
for batch in [
SCHEMA_V1, SCHEMA_V2, SCHEMA_V3, SCHEMA_V4, SCHEMA_V5, SCHEMA_V6, SCHEMA_V7,
] {
conn.execute_batch(batch).expect("batch");
}
conn.execute_batch("PRAGMA user_version = 7;").expect("v7");
conn
}
fn add_note(conn: &Connection, id: &str, body: &str) {
conn.execute(
"INSERT INTO notes (id, body, created_at, updated_at)
VALUES (?1, ?2, '2026-01-01T00:00:00.000Z', '2026-01-01T00:00:00.000Z')",
params![id, body],
)
.expect("note");
}
fn add_item(conn: &Connection, note: &str, text: &str, checked: bool, pos: i64) {
conn.execute(
"INSERT INTO checklist_items (id, note_id, text, checked, position)
VALUES (?1, ?2, ?3, ?4, ?5)",
params![format!("{note}-{pos}"), note, text, checked, pos],
)
.expect("item");
}
fn body_of(conn: &Connection, id: &str) -> String {
conn.query_row("SELECT body FROM notes WHERE id = ?1", [id], |r| r.get(0))
.expect("body")
}
#[test]
fn v8_folds_items_into_the_body() {
let conn = v7_db();
add_note(&conn, "n1", "shopping");
add_item(&conn, "n1", "milk", false, 0);
add_item(&conn, "n1", "eggs", true, 1);
migrate(&conn).expect("migrate");
// Prose, blank line, list — the layout _note_markdown already exports, so an
// export taken before this migration and one taken after agree byte for byte.
assert_eq!(body_of(&conn, "n1"), "shopping\n\n- [ ] milk\n- [x] eggs");
}
#[test]
fn v8_keeps_a_list_only_note_whole() {
let conn = v7_db();
add_note(&conn, "n1", "");
add_item(&conn, "n1", "milk", false, 0);
migrate(&conn).expect("migrate");
assert_eq!(body_of(&conn, "n1"), "- [ ] milk");
}
#[test]
fn v8_leaves_timestamps_alone() {
// The whole reason this needs no sync: the server folds the same rows the same
// way, so both sides already agree. Marking notes dirty would push a body the
// server has, from every device at once.
let conn = v7_db();
add_note(&conn, "n1", "note");
add_item(&conn, "n1", "milk", false, 0);
migrate(&conn).expect("migrate");
let (updated, dirty): (String, i64) = conn
.query_row(
"SELECT updated_at, dirty FROM notes WHERE id = 'n1'",
[],
|r| Ok((r.get(0)?, r.get(1)?)),
)
.expect("row");
assert_eq!(updated, "2026-01-01T00:00:00.000Z");
assert_eq!(dirty, 1); // as inserted, not raised by the migration
}
#[test]
fn v8_drops_the_table_and_is_idempotent() {
let conn = v7_db();
add_note(&conn, "n1", "note");
migrate(&conn).expect("migrate");
migrate(&conn).expect("again");
let exists: i64 = conn
.query_row(
"SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name='checklist_items'",
[],
|r| r.get(0),
)
.expect("count");
assert_eq!(exists, 0);
}
#[test]
fn a_fresh_database_reaches_v8() {
let conn = Connection::open_in_memory().expect("open");
migrate(&conn).expect("migrate");
let version: i64 = conn
.query_row("PRAGMA user_version", [], |r| r.get(0))
.expect("version");
assert_eq!(version, 8);
}
}
+84 -65
View File
@@ -9,7 +9,7 @@
use chrono::{DateTime, Duration, SecondsFormat, Utc};
use rusqlite::{params, params_from_iter, Connection, OptionalExtension};
use serde_json::Value;
use serde_json::{json, Value};
use uuid::Uuid;
use crate::local::derive;
@@ -24,24 +24,25 @@ fn new_id() -> String {
Uuid::new_v4().to_string()
}
/// The note's NAME: its first non-blank body line, else its first checklist item.
/// The note's NAME: the first line of its body that says anything.
///
/// Mirrors `derive_display_title` in the server's notes/helpers.py — one rule written
/// twice, and they have to agree or a synced note is called different things on either
/// side of the wire.
///
/// Pure, and given the items rather than fetching them: every caller has already
/// loaded them, so a query here would be a second trip for something already in hand.
fn display_title(body: &str, items: &[ChecklistItem]) -> String {
if let Some(line) = body.lines().map(str::trim).find(|l| !l.is_empty()) {
return line.to_string();
/// It no longer needs the items, because the items ARE lines of the body now (M304).
/// What it needs instead is to strip the task marker off: a list-only note is still
/// named by its first item, and calling that note "- [ ] milk" would be showing
/// someone the storage rather than the note. An empty item is skipped rather than
/// naming the note "", which is what a half-typed list would otherwise do.
fn display_title(body: &str) -> String {
for line in body.lines() {
let text = derive::strip_marker(line.trim()).trim();
if !text.is_empty() {
return text.to_string();
}
}
items
.iter()
.map(|i| i.text.trim())
.find(|t| !t.is_empty())
.unwrap_or("")
.to_string()
String::new()
}
fn escape_like(s: &str) -> String {
@@ -69,19 +70,24 @@ fn load_labels(conn: &Connection, note_id: &str) -> rusqlite::Result<Vec<NoteLab
rows.collect()
}
fn load_items(conn: &Connection, note_id: &str) -> rusqlite::Result<Vec<ChecklistItem>> {
let mut stmt = conn.prepare(
"SELECT id, text, checked, position FROM checklist_items WHERE note_id = ?1 ORDER BY position ASC",
)?;
let rows = stmt.query_map([note_id], |r| {
Ok(ChecklistItem {
id: r.get(0)?,
text: r.get(1)?,
checked: r.get(2)?,
position: r.get(3)?,
/// The note's checklist, read out of its body. No query, because there is no table.
///
/// A `- [ ] milk` line IS the item (M304). The id is the item's ORDINAL rather than a
/// uuid — which is all it ever amounted to anyway, since `push.rs` sent text and
/// checked and never an id, and both sides replaced the whole list on every sync. It
/// is also exactly what the rewriters in `derive` take, so a UI holding an id can act
/// on it directly.
fn items_of(body: &str) -> Vec<ChecklistItem> {
derive::extract_items(body)
.into_iter()
.enumerate()
.map(|(i, item)| ChecklistItem {
id: i.to_string(),
text: item.text,
checked: item.checked,
position: i as i64,
})
})?;
rows.collect()
.collect()
}
fn load_attachments(conn: &Connection, note_id: &str) -> rusqlite::Result<Vec<Attachment>> {
@@ -162,11 +168,10 @@ fn load_note(conn: &Connection, id: &str) -> rusqlite::Result<Note> {
},
)?;
note.labels = load_labels(conn, id)?;
note.items = load_items(conn, id)?;
note.items = items_of(&note.body);
note.attachments = load_attachments(conn, id)?;
note.previews = load_previews(conn, id)?;
// After the items, because a body-only-empty note is named by its first one.
note.display_title = display_title(&note.body, &note.items);
note.display_title = display_title(&note.body);
Ok(note)
}
@@ -358,20 +363,22 @@ pub fn create_note(conn: &Connection, input: &NoteCreateInput) -> rusqlite::Resu
[],
|r| r.get(0),
)?;
// Items fold into the body rather than into rows of their own. Callers still hand
// them over separately — the importer has a list, not a blob — but where they end
// up is one place.
let mut body = input.body.clone();
if let Some(items) = &input.items {
for text in items {
body = derive::append_item(&body, text, false);
}
}
conn.execute(
"INSERT INTO notes (id, body, color, position, created_at, updated_at, dirty)
VALUES (?1, ?2, ?3, ?4, ?5, ?5, 1)",
params![id, input.body, input.color, position, ts],
params![id, body, input.color, position, ts],
)?;
if let Some(items) = &input.items {
for (i, text) in items.iter().enumerate() {
conn.execute(
"INSERT INTO checklist_items (id, note_id, text, position) VALUES (?1, ?2, ?3, ?4)",
params![new_id(), id, text, i as i64],
)?;
}
}
sync_tags(conn, &id, &input.body)?;
// The FOLDED body, not the input one: an item can carry a #tag too.
sync_tags(conn, &id, &body)?;
load_note(conn, &id)
}
@@ -548,18 +555,32 @@ pub fn set_labels(conn: &Connection, id: &str, label_ids: &[String]) -> rusqlite
load_note(conn, id)
}
// ---- checklist items: every one of these is a body edit ---------------------
//
// They keep their own names and signatures because the FFI, the Tauri commands and
// the REST shape all speak in items, and a checklist is still a thing a note HAS.
// What changed is where it is kept. Routing all three through `update_note` rather
// than writing the body directly is what gives them revision snapshotting, `#tag`
// re-derivation and the dirty/updated_at bookkeeping without any of it being
// written a second time here.
fn note_body(conn: &Connection, id: &str) -> rusqlite::Result<String> {
conn.query_row("SELECT body FROM notes WHERE id = ?1", [id], |r| r.get(0))
}
/// An item's id is its ordinal (see [items_of]). Anything else is a stale id from a
/// UI that has not reloaded, and the right answer to those is to do nothing.
fn item_index(item_id: &str) -> Option<usize> {
item_id.parse::<usize>().ok()
}
fn set_body(conn: &Connection, id: &str, body: String) -> rusqlite::Result<Note> {
update_note(conn, id, &json!({ "body": body }))
}
pub fn add_item(conn: &Connection, id: &str, text: &str) -> rusqlite::Result<Note> {
let pos: i64 = conn.query_row(
"SELECT COALESCE(MAX(position), -1) + 1 FROM checklist_items WHERE note_id = ?1",
[id],
|r| r.get(0),
)?;
conn.execute(
"INSERT INTO checklist_items (id, note_id, text, position) VALUES (?1, ?2, ?3, ?4)",
params![new_id(), id, text, pos],
)?;
touch(conn, id)?;
load_note(conn, id)
let body = note_body(conn, id)?;
set_body(conn, id, derive::append_item(&body, text, false))
}
pub fn update_item(
@@ -568,29 +589,27 @@ pub fn update_item(
item_id: &str,
changes: &Value,
) -> rusqlite::Result<Note> {
let index = match item_index(item_id) {
Some(i) => i,
None => return load_note(conn, id),
};
let mut body = note_body(conn, id)?;
if let Some(text) = changes.get("text").and_then(Value::as_str) {
conn.execute(
"UPDATE checklist_items SET text = ?1 WHERE id = ?2 AND note_id = ?3",
params![text, item_id, id],
)?;
body = derive::set_item_text(&body, index, text);
}
if let Some(checked) = changes.get("checked").and_then(Value::as_bool) {
conn.execute(
"UPDATE checklist_items SET checked = ?1 WHERE id = ?2 AND note_id = ?3",
params![checked, item_id, id],
)?;
body = derive::set_item_checked(&body, index, checked);
}
touch(conn, id)?;
load_note(conn, id)
set_body(conn, id, body)
}
pub fn delete_item(conn: &Connection, id: &str, item_id: &str) -> rusqlite::Result<Note> {
conn.execute(
"DELETE FROM checklist_items WHERE id = ?1 AND note_id = ?2",
params![item_id, id],
)?;
touch(conn, id)?;
load_note(conn, id)
let index = match item_index(item_id) {
Some(i) => i,
None => return load_note(conn, id),
};
let body = note_body(conn, id)?;
set_body(conn, id, derive::remove_item(&body, index))
}
pub fn delete_attachment(conn: &Connection, id: &str, att_id: &str) -> rusqlite::Result<Note> {