refactor: normalize review and comment storage

README.md

@@ -13,19 +13,22 @@ cargo run -- --config examples/config.rb notes.txt
 
 Without a file, vamp opens an unnamed buffer. A nonexistent file opens an empty buffer. `cargo run -- --help` lists the command-line options.
 
-### Read local reviews without opening the editor
+### Manage local reviews without opening the editor
 
 ```sh
 vamp --review list --repo .
 vamp --review list --repo . --json
 vamp --review show 1 --repo . --json
 vamp --review show 1 --repo . --all
+vamp --review delete 1 3 --repo . # Delete comment 3 from review 1
 ```
 
 The review CLI discovers the Git/Jujutsu checkout from `--repo` (default: current directory). It uses the same default configuration, `--config PATH`, `--no-config`, and `review_directory` as the editor, but starts no editor workers or terminal UI. Missing storage lists no reviews without creating a database; incompatible storage is rejected and preserved.
 
 `show` includes unresolved comments by default; `--all` includes resolved comments too. Human-readable output is the default. JSON includes the canonical repository root, storage review ID/revision, summary/verdict, total/resolved/unresolved counts, and comments with exact zero-based, end-exclusive grapheme anchors and selected source excerpts—not whole source files. `list --json` returns the repository and review metadata/counts without comments. Explicit editor JSON exports still contain complete anchor source text.
 
+`list` and `show` are read-only. `delete REVIEW_ID COMMENT_ID` immediately commits deletion of one resolved or unresolved comment, then prints the updated review with its remaining unresolved comments (`--json` is supported). Both IDs are required; missing reviews/comments and repository mismatches fail without changing stored reviews or creating missing storage. Deleting the last comment keeps the review, summary, and verdict. Concurrent saves use the same revision-conflict protection as the editor; an already-open editor must reload before saving after a CLI deletion. Existing JSON exports are not modified.
+
 `vamp review` still opens a file named `review`; `vamp -- --review` opens a file named `--review`.
 
 ## Getting started

docs/architecture.md

@@ -17,8 +17,9 @@ One editor crate; the workspace also includes its patched Crossterm dependency.
   Virtual diff leaves retain their original catalog input index for caller-owned annotations. Normal scanned leaves accept optional repository status and old-path metadata without filtering unchanged files; missing tracked deletions add virtual leaves and required directories. Metadata-only updates preserve query, selection, expansion, and view without rebuilding the catalog.
 - `diff`: owns searchable Before/After selection, category, focus and explorer visibility, request freshness, read-only comparison DTOs, aligned rows with cached grapheme-level change ranges, and shared vertical/independent horizontal offsets. Changed paths form a virtual `files::FilePicker` catalog, reusing the normal tree projection, filtering, and navigation without scanning disk. It has no subprocess or terminal dependencies.
 - `repository`: discovers jj/Git repositories and local/advertised remote revisions, builds disk snapshots and changed-file hunks, and owns isolated temporary comparison repositories with bounded, cancellable subprocesses. It never saves editor buffers or modifies active refs/checkouts.
-- `review`: owns the validated local review model, exact immutable anchors, SQLite storage through bundled rusqlite, and atomic JSON import/export with external-change protection. `Store` uses schema-versioned transactions and compare-and-swap review revisions; repository roots and persistent review IDs are independent of comparison revisions. One bounded JSON document is stored per row, retaining exact grapheme endpoints and stale snapshots. Incompatible schemas and concurrent edits fail without replacing data or advancing in-memory saved state.
+- `review`: owns the validated local review model, exact immutable anchors, SQLite storage through bundled rusqlite, and atomic JSON import/export with external-change protection. Schema 2 stores review identity, repository, revision, summary, and verdict in `reviews`; `comments` stores ordered comment IDs, bodies, resolution states, and explicit immutable anchor fields. Foreign keys, unique keys, and checks enforce relationships and scalar invariants. Full `u64` comment IDs use checked decimal text because SQLite integers are signed. No JSON documents are stored in SQLite.
-  Store listing returns validated review documents in descending ID order. CLI metadata, sole-review restoration, and selector labels reuse those documents without reloading them; selector choices live only in the displayed buffer.
+  `Store` retains compare-and-swap review revisions and atomic whole-review saves: metadata and replacement comment rows commit together. Read transactions reconstruct validated reviews from one consistent snapshot, including when listing in descending ID order. Stored text is bounded to 64 MiB per review, including a fixed per-comment overhead. JSON version 3 is an independent import/export format. Incompatible databases, including schema 1 document storage, are rejected and preserved without migration or reset. CLI metadata, sole-review restoration, and selector labels reuse loaded reviews; selector choices live only in the displayed buffer.
+  The review CLI keeps `list`/`show` read-only. Comment deletion validates the explicit review/comment IDs through an existing read-only store before opening it for a compare-and-swap save; absent storage and invalid targets do not create reviews or a database.
   `editor_review` commits native comment/summary buffers on dismissal or switching, and verdicts/deletions immediately. It restores a sole stored review or opens the floating selector for multiple reviews. Exports retain their own file baseline and do not become automatic persistence targets; imports get fresh database identities. Review editors remain tab-owned native panes outside the workspace `PaneTree`. Storage opens lazily when a comparison is ready; the session supplies the configured or XDG default directory. Direct `Editor` callers inject a directory with `set_review_directory` to enable persistence.
   Successful SQLite autosaves and JSON-only saves share one acknowledgement operation: clear review dirtiness and advance the owning tab's native draft saved baselines only after persistence succeeds. Failed writes retain unsaved state; JSON exports do not acknowledge a pending SQLite save.
   Review replacement and comment deletion are private editor-owned operations. Replacement loads and validates its Stored/JSON/New source before retiring old bindings, installing the document, and resetting selection/navigation. Deletion retires and acknowledges only the matching comment drafts, preventing stale text from overwriting reused IDs. Both persist before opening the overview; a post-mutation save failure retains the applied, unsaved identity change. Native text/undo and general transition guards remain unchanged.

docs/development.md

@@ -70,10 +70,12 @@ Diff checks cover staged category/filter navigation, stale request cancellation,
 
 Local review checks cover native comment/summary composition and undo, Visual ranges on either side, verdicts, JSON save/reopen, external-write refusal, hidden drafts and teardown guards, immutable Working identities in Git/jj, renamed paths, live EOF, and stale anchors. Render checks cover selected/range markers, split/unified context, wrapped continuations, and narrow gutters. Live PTY smoke creates and saves a comment/summary/verdict, closes and reopens the review, edits the reopened comment, changes the reviewed source, and observes stale status without altering the saved anchor. Fixtures are local repositories; no forge writes.
 
-SQLite review checks cover exact anchors across restarts and revision changes, repository isolation, independent connections and optimistic conflicts, locked/unwritable stores, incompatible schema preservation, bounded/corrupt documents, private file permissions, and XDG/config path resolution. Editor checks require explicit writes for comment/summary text, discard unsaved text on dismissal without retaining discarded undo history, prevent metadata autosaves from committing hidden drafts, persist verdicts/deletions, restore a sole review, choose among multiple reviews, and keep exports independent from autosaves. Test fixtures use isolated review directories. Live PTY smoke verifies that Escape leaves no new stored comment, `:w` persists exact text, and pane close discards a later edit; reopening and writing again preserves the saved body.
+SQLite review checks cover exact anchors across restarts and revision changes, repository isolation, independent connections and optimistic conflicts, locked/unwritable stores, incompatible schema preservation, relational constraints, full unsigned comment IDs, stable comment order, bounded/corrupt rows, rollback after partial comment insertion, private file permissions, and XDG/config path resolution. Schema 1 document storage is rejected without modification. Editor checks require explicit writes for comment/summary text, discard unsaved text on dismissal without retaining discarded undo history, prevent metadata autosaves from committing hidden drafts, persist verdicts/deletions, restore a sole review, choose among multiple reviews, and keep exports independent from autosaves. Test fixtures use isolated review directories. Live PTY smoke verifies that Escape leaves no new stored comment, `:w` persists exact text, and pane close discards a later edit; reopening and writing again preserves the saved body.
 
 Manual-resolution checks cover immediate persistence, reopen/export/import, independent stale status, conflict-retained drafts, and incompatible version preservation. CLI checks cover filename-safe dispatch, Unicode excerpts, unresolved filtering/counts, absent storage, read-only access and permissions, symlink rejection, and incompatible schema preservation.
 Live PTY/CLI smoke composes and explicitly saves a comment, resolves it in the editor, observes an empty default CLI comment list with explicit counts, retrieves the resolved comment and exact excerpt with `--all`, and unresolves it. Each transition advances the same stored review revision; listing absent storage creates no data directory.
+CLI deletion checks exercise explicit review/comment IDs, resolved-comment deletion, repository isolation, preserved summaries/verdicts/other comments, last-comment deletion, missing targets/storage, and stale-save conflicts through the executable. CLI smoke verifies persisted deletion and rejects a repeated deletion without advancing the review revision.
+Relational-storage smoke inspects typed review/comment columns, round-trips exact Unicode anchors through the store and JSON export/import, deletes through the real CLI, and confirms the stored review and existing export remain intact.
 
 
 Conflicted-revision regressions preserve unchanged symlinks, defer oversized unselected blobs until file selection, compare selected conflict text with jj's projection, and exclude unchanged conflicts when comparing later revisions. Live terminal smoke covers local bookmark selection and opening an actual conflicted file without changing the checkout.

docs/editing.md

@@ -487,11 +487,15 @@ Local review buffers (comments, summaries, overview, and stored-review selector)
 
 Comment and summary text is committed only by **`:w`** or **`:diff.review.save`**, which saves the entire review, including hidden drafts. Dismissing an editor discards its unsaved text; switching between review editors can retain hidden drafts but does not save them. Verdicts, deletions, and manual resolution changes commit immediately to local SQLite without committing draft text. `:diff.review.save path` or `:w path` additionally exports JSON; later metadata autosaves still go to SQLite and leave that export unchanged. Imports create a new stored review identity rather than overwriting another review.
 
+From the shell, use `vamp --review delete REVIEW_ID COMMENT_ID [--repo PATH] [--json]` to delete a stored comment immediately without opening the editor. Find IDs with `vamp --review list` and `vamp --review show REVIEW_ID --all`. The repository defaults to the current checkout. Both resolved and unresolved comments can be deleted; the review and any JSON exports remain intact. An editor holding the old review revision must reload before saving.
+
 Storage defaults to `$XDG_DATA_HOME/vamp/reviews/reviews.sqlite3`, or `~/.local/share/vamp/reviews/reviews.sqlite3` when XDG_DATA_HOME is unset or relative. Configure `review_directory` to override its directory; relative configuration paths resolve against the config file. Repository identity is the canonical checkout root. Each review has a persistent database ID independent of comparison revisions, so reopening changed revisions retains stale comments. Opening a comparison automatically restores the repository's sole review. Multiple reviews open a floating selector: move to an entry and press Enter, or use `:diff.review.open id`. Use `:diff.review.new` for another independent review.
 
 Write errors, locked databases, incompatible schemas, and conflicting edits from another editor instance retain dirty drafts and report the failure. Failed text saves leave the committed review unchanged; unrelated metadata autosaves cannot commit those drafts. Normal destructive closes remain guarded until drafts are explicitly saved or discarded. `:w!` does not bypass database revision conflicts; export your draft before reopening the current stored review. JSON exports retain external-file-change protection. Only the current database schema is accepted; existing incompatible databases are preserved rather than reset or migrated.
 
-Version 3 JSON is the agent-readable sharing format: repository root, summary, optional verdict, and comments with stable IDs, bodies, required boolean `resolved` states, and anchors. New comments are unresolved. Database review IDs and optimistic storage revisions stay internal. Each anchor retains its immutable comparison revision, side, relative path (old path for renamed Before files), and exact whole-file text including the final newline. `start` and `end` contain zero-based source `row` and extended-grapheme `column` coordinates; `end` is exclusive. Columns count complete Unicode graphemes, not bytes, code points, UTF-16 units, or screen cells. Overview/window labels display one-based rows and columns. Older formats, including version 2, are rejected and preserved without migration. Working comparisons use content-addressed snapshot IDs; captured text additionally freezes live unsaved edits. Each stored or exported review is limited to 64 MiB.
+The current SQLite format is schema 2: typed `reviews` and `comments` tables, not serialized JSON documents. Schema 1 databases are incompatible and are left untouched; there is no migration or automatic reset. JSON version 3 remains the sharing format, independent of the database schema.
+
+Version 3 JSON is the agent-readable sharing format: repository root, summary, optional verdict, and comments with stable IDs, bodies, required boolean `resolved` states, and anchors. New comments are unresolved. Database review IDs and optimistic storage revisions stay internal. Each anchor retains its immutable comparison revision, side, relative path (old path for renamed Before files), and exact whole-file text including the final newline. `start` and `end` contain zero-based source `row` and extended-grapheme `column` coordinates; `end` is exclusive. Columns count complete Unicode graphemes, not bytes, code points, UTF-16 units, or screen cells. Overview/window labels display one-based rows and columns. Older JSON formats, including version 2, are rejected and preserved without migration. Working comparisons use content-addressed snapshot IDs; captured text additionally freezes live unsaved edits. Stored reviews have a 64 MiB aggregate text budget including 64 bytes of overhead per comment; imported/exported JSON files have a separate 64 MiB serialized-size limit.
 
 Reopening requires the same repository root. Comments whose revision, path, or exact file text no longer matches are **stale**: their original anchors remain unchanged and their gutter markers disappear. Unloaded files are shown as **not checked** until visited; refresh the overview to update its statuses. There is no automatic reanchoring, server, authentication, or review-publishing operation.
 

src/main.rs

@@ -12,8 +12,10 @@ const HELP: &str = "vamp — modal editor with isolated Ruby plugins
 Usage: vamp [--config PATH | --no-config] [FILE]
        vamp [--config PATH | --no-config] --review list [--repo PATH] [--json]
        vamp [--config PATH | --no-config] --review show ID [--repo PATH] [--json] [--all]
+       vamp [--config PATH | --no-config] --review delete REVIEW_ID COMMENT_ID [--repo PATH] [--json]
 
-Review CLI: read-only local storage; show defaults to unresolved comments; --all includes resolved.
+Review CLI: list/show are read-only; delete commits one comment deletion immediately.
+Show defaults to unresolved comments; --all includes resolved.
 Use vamp -- --review to edit a file named --review.
 
 Default keys (configurable with config.bind):

src/review.rs

@@ -281,12 +281,36 @@ fn directory_from(xdg: Option<&OsStr>, home: Option<&OsStr>) -> Result<PathBuf,
         .ok_or_else(|| "Cannot locate review storage: HOME must be an absolute directory".into())
 }
 
-// ponytail: whole-review JSON commits; normalize comment rows if large reviews make writes costly.
+const REVIEWS_SCHEMA: &str = "CREATE TABLE reviews (
-const SCHEMA: &str = "CREATE TABLE reviews (
     id INTEGER PRIMARY KEY CHECK (id > 0),
     root TEXT NOT NULL,
     revision INTEGER NOT NULL CHECK (revision > 0),
-    document TEXT NOT NULL CHECK (length(CAST(document AS BLOB)) <= 67108864)
+    summary TEXT NOT NULL,
+    verdict TEXT CHECK (verdict IN ('looks_good', 'changes_requested'))
+) STRICT";
+
+// SQLite integers are signed; decimal text preserves the model's full u64 comment IDs.
+const COMMENTS_SCHEMA: &str = "CREATE TABLE comments (
+    review_id INTEGER NOT NULL REFERENCES reviews(id) ON DELETE CASCADE,
+    id TEXT NOT NULL CHECK (
+        length(id) BETWEEN 1 AND 20 AND id NOT GLOB '*[^0-9]*'
+        AND substr(id, 1, 1) BETWEEN '1' AND '9'
+        AND (length(id) < 20 OR id <= '18446744073709551615')
+    ),
+    position INTEGER NOT NULL CHECK (position >= 0),
+    body TEXT NOT NULL,
+    resolved INTEGER NOT NULL CHECK (resolved IN (0, 1)),
+    anchor_revision TEXT NOT NULL CHECK (length(anchor_revision) > 0),
+    path TEXT NOT NULL,
+    side TEXT NOT NULL CHECK (side IN ('before', 'after')),
+    start_row INTEGER NOT NULL CHECK (start_row >= 0),
+    start_column INTEGER NOT NULL CHECK (start_column >= 0),
+    end_row INTEGER NOT NULL CHECK (end_row >= 0),
+    end_column INTEGER NOT NULL CHECK (end_column >= 0),
+    content TEXT NOT NULL,
+    PRIMARY KEY (review_id, id),
+    UNIQUE (review_id, position),
+    CHECK (end_row > start_row OR (end_row = start_row AND end_column >= start_column))
 ) STRICT";
 
 pub struct Store {
@@ -316,6 +340,9 @@ impl Store {
         )
         .map_err(database_error)?;
         Self::check_schema(&connection)?;
+        connection
+            .pragma_update(None, "foreign_keys", true)
+            .map_err(database_error)?;
         Ok(Some(Self { connection }))
     }
 
@@ -330,7 +357,7 @@ impl Store {
             .map_err(database_error)?
             .collect::<Result<_, _>>()
             .map_err(database_error)?;
-        if version != 1 || schema != [SCHEMA] {
+        if version != 2 || schema != [COMMENTS_SCHEMA, REVIEWS_SCHEMA] {
             return Err(format!(
                 "Unsupported review database schema (version {version}); existing data was not changed"
             ));
@@ -378,6 +405,9 @@ impl Store {
                 | OpenFlags::SQLITE_OPEN_NOFOLLOW,
         )
         .map_err(database_error)?;
+        connection
+            .pragma_update(None, "foreign_keys", true)
+            .map_err(database_error)?;
         connection
             .busy_timeout(std::time::Duration::ZERO)
             .map_err(database_error)?;
@@ -385,9 +415,14 @@ impl Store {
             let transaction = connection
                 .transaction_with_behavior(TransactionBehavior::Immediate)
                 .map_err(database_error)?;
-            transaction.execute_batch(SCHEMA).map_err(database_error)?;
             transaction
-                .pragma_update(None, "user_version", 1)
+                .execute_batch(REVIEWS_SCHEMA)
+                .map_err(database_error)?;
+            transaction
+                .execute_batch(COMMENTS_SCHEMA)
+                .map_err(database_error)?;
+            transaction
+                .pragma_update(None, "user_version", 2)
                 .map_err(database_error)?;
             transaction.commit().map_err(database_error)?;
         } else {
@@ -407,17 +442,24 @@ impl Store {
         let root_text = root
             .to_str()
             .ok_or("Review repository root must be UTF-8")?;
-        let mut statement = self
+        let transaction = self
             .connection
+            .unchecked_transaction()
+            .map_err(database_error)?;
+        let reviews = {
+            let mut statement = transaction
                 .prepare(
-                "SELECT id, root, revision, document FROM reviews WHERE root = ?1 ORDER BY id DESC",
+                    "SELECT id, root, revision, summary, verdict FROM reviews WHERE root = ?1 ORDER BY id DESC",
                 )
                 .map_err(database_error)?;
             let mut rows = statement.query([root_text]).map_err(database_error)?;
             let mut reviews = Vec::new();
             while let Some(row) = rows.next().map_err(database_error)? {
-            reviews.push(stored_review(row, &root)?);
+                reviews.push(stored_review(&transaction, row, &root)?);
             }
+            reviews
+        };
+        transaction.commit().map_err(database_error)?;
         Ok(reviews)
     }
 
@@ -426,9 +468,13 @@ impl Store {
         let root_text = root
             .to_str()
             .ok_or("Review repository root must be UTF-8")?;
-        let mut statement = self
+        let transaction = self
             .connection
-            .prepare("SELECT id, root, revision, document FROM reviews WHERE id = ?1 AND root = ?2")
+            .unchecked_transaction()
+            .map_err(database_error)?;
+        let review = {
+            let mut statement = transaction
+                .prepare("SELECT id, root, revision, summary, verdict FROM reviews WHERE id = ?1 AND root = ?2")
                 .map_err(database_error)?;
             let mut rows = statement
                 .query(params![id, root_text])
@@ -437,7 +483,10 @@ impl Store {
                 .next()
                 .map_err(database_error)?
                 .ok_or("Review does not exist in this repository")?;
-        stored_review(row, &root)
+            stored_review(&transaction, row, &root)?
+        };
+        transaction.commit().map_err(database_error)?;
+        Ok(review)
     }
 
     pub fn save(&mut self, review: &mut Review) -> Result<(), String> {
@@ -457,19 +506,34 @@ impl Store {
             None if review.storage_revision == 0 => 1,
             _ => return Err("Invalid review storage identity".into()),
         };
-        let bytes = serde_json::to_vec(review).map_err(|error| error.to_string())?;
+        let mut size = review.root.as_os_str().len() + review.summary.len();
-        if bytes.len() > LIMIT {
+        for comment in &review.comments {
+            size = size
+                .checked_add(
+                    comment.body.len()
+                        + comment.anchor.revision.len()
+                        + comment.anchor.path.as_os_str().len()
+                        + comment.anchor.content.len()
+                        + 64,
+                )
+                .ok_or("Review exceeds the 64 MiB limit")?;
+        }
+        if size > LIMIT {
             return Err("Review exceeds the 64 MiB limit".into());
         }
-        let document = std::str::from_utf8(&bytes).map_err(|error| error.to_string())?;
+        let verdict = match review.verdict {
+            Some(Verdict::LooksGood) => Some("looks_good"),
+            Some(Verdict::ChangesRequested) => Some("changes_requested"),
+            None => None,
+        };
         let transaction = self
             .connection
             .transaction_with_behavior(TransactionBehavior::Immediate)
             .map_err(database_error)?;
         let id = if let Some(id) = review.storage_id {
             let changed = transaction.execute(
-                "UPDATE reviews SET revision = ?1, document = ?2 WHERE id = ?3 AND root = ?4 AND revision = ?5",
+                "UPDATE reviews SET revision = ?1, summary = ?2, verdict = ?3 WHERE id = ?4 AND root = ?5 AND revision = ?6",
-                params![revision, document, id, root, review.storage_revision],
+                params![revision, review.summary, verdict, id, root, review.storage_revision],
             ).map_err(database_error)?;
             if changed != 1 {
                 return Err(
@@ -480,12 +544,46 @@ impl Store {
         } else {
             transaction
                 .execute(
-                    "INSERT INTO reviews (root, revision, document) VALUES (?1, ?2, ?3)",
+                    "INSERT INTO reviews (root, revision, summary, verdict) VALUES (?1, ?2, ?3, ?4)",
-                    params![root, revision, document],
+                    params![root, revision, review.summary, verdict],
                 )
                 .map_err(database_error)?;
             transaction.last_insert_rowid()
         };
+        // ponytail: replace comment rows per save; diff rows if snapshot writes become costly.
+        transaction
+            .execute("DELETE FROM comments WHERE review_id = ?1", [id])
+            .map_err(database_error)?;
+        {
+            let mut insert = transaction.prepare(
+                "INSERT INTO comments (review_id, id, position, body, resolved, anchor_revision, path, side,
+                    start_row, start_column, end_row, end_column, content)
+                 VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13)",
+            ).map_err(database_error)?;
+            for (position, comment) in review.comments.iter().enumerate() {
+                let anchor = &comment.anchor;
+                insert
+                    .execute(params![
+                        id,
+                        comment.id.to_string(),
+                        sql_index(position)?,
+                        comment.body,
+                        comment.resolved,
+                        anchor.revision,
+                        anchor.path.to_str().unwrap(),
+                        match anchor.side {
+                            Side::Before => "before",
+                            Side::After => "after",
+                        },
+                        sql_index(anchor.start.row)?,
+                        sql_index(anchor.start.column)?,
+                        sql_index(anchor.end.row)?,
+                        sql_index(anchor.end.column)?,
+                        anchor.content,
+                    ])
+                    .map_err(database_error)?;
+            }
+        }
         transaction.commit().map_err(database_error)?;
         review.storage_id = Some(id);
         review.storage_revision = revision;
@@ -497,26 +595,111 @@ fn database_error(error: rusqlite::Error) -> String {
     format!("Review database: {error}")
 }
 
-fn stored_review(row: &rusqlite::Row<'_>, root: &Path) -> Result<Review, String> {
+fn sql_index(value: usize) -> Result<i64, String> {
+    i64::try_from(value).map_err(|_| "Comment coordinate exceeds SQLite's integer range".into())
+}
+
+fn stored_index(row: &rusqlite::Row<'_>, index: usize) -> Result<usize, String> {
+    usize::try_from(row.get::<_, i64>(index).map_err(database_error)?)
+        .map_err(|_| "Stored comment coordinate exceeds this platform's range".into())
+}
+
+fn stored_review(
+    connection: &Connection,
+    row: &rusqlite::Row<'_>,
+    root: &Path,
+) -> Result<Review, String> {
     let id: i64 = row.get(0).map_err(database_error)?;
-    let stored_root: String = row.get(1).map_err(database_error)?;
+    let stored_root = row
+        .get_ref(1)
+        .map_err(database_error)?
+        .as_str()
+        .map_err(|error| error.to_string())?;
     let revision: i64 = row.get(2).map_err(database_error)?;
-    if id <= 0 || revision <= 0 || Path::new(&stored_root) != root {
+    if id <= 0 || revision <= 0 || Path::new(stored_root) != root {
         return Err("Invalid review database identity".into());
     }
-    let document = row.get_ref(3).map_err(database_error)?;
+    let summary = row
-    let bytes = document
+        .get_ref(3)
+        .map_err(database_error)?
         .as_str()
-        .map_err(|error| format!("Invalid stored review: {error}"))?
+        .map_err(|error| error.to_string())?;
-        .as_bytes();
+    let comment_bytes: i64 = connection
-    if bytes.len() > LIMIT {
+        .query_row(
+            "SELECT COALESCE(SUM(length(CAST(body AS BLOB)) + length(CAST(anchor_revision AS BLOB))
+            + length(CAST(path AS BLOB)) + length(CAST(content AS BLOB)) + 64), 0)
+         FROM comments WHERE review_id = ?1",
+            [id],
+            |row| row.get(0),
+        )
+        .map_err(database_error)?;
+    if comment_bytes < 0
+        || comment_bytes as u64 + stored_root.len() as u64 + summary.len() as u64 > LIMIT as u64
+    {
         return Err("Review exceeds the 64 MiB limit".into());
     }
-    let mut review = Review::from_json(bytes)?;
+    let mut review = Review {
-    review.validate()?;
+        root: root.to_owned(),
-    if review.root != root {
+        ..Review::default()
-        return Err("Stored review belongs to a different repository".into());
+    };
+    review.summary = summary.to_owned();
+    review.verdict = match row
+        .get::<_, Option<String>>(4)
+        .map_err(database_error)?
+        .as_deref()
+    {
+        Some("looks_good") => Some(Verdict::LooksGood),
+        Some("changes_requested") => Some(Verdict::ChangesRequested),
+        None => None,
+        _ => return Err("Invalid stored review verdict".into()),
+    };
+    let mut statement = connection
+        .prepare(
+            "SELECT id, position, body, resolved, anchor_revision, path, side,
+            start_row, start_column, end_row, end_column, content
+         FROM comments WHERE review_id = ?1 ORDER BY position",
+        )
+        .map_err(database_error)?;
+    let mut rows = statement.query([id]).map_err(database_error)?;
+    while let Some(row) = rows.next().map_err(database_error)? {
+        if stored_index(row, 1)? != review.comments.len() {
+            return Err("Invalid stored comment order".into());
         }
+        let side = match row.get::<_, String>(6).map_err(database_error)?.as_str() {
+            "before" => Side::Before,
+            "after" => Side::After,
+            _ => return Err("Invalid stored comment side".into()),
+        };
+        let resolved = match row.get::<_, i64>(3).map_err(database_error)? {
+            0 => false,
+            1 => true,
+            _ => return Err("Invalid stored comment resolution".into()),
+        };
+        review.comments.push(Comment {
+            id: row
+                .get::<_, String>(0)
+                .map_err(database_error)?
+                .parse::<u64>()
+                .map_err(|_| "Invalid stored comment ID")?,
+            body: row.get(2).map_err(database_error)?,
+            resolved,
+            anchor: Anchor {
+                revision: row.get(4).map_err(database_error)?,
+                path: PathBuf::from(row.get::<_, String>(5).map_err(database_error)?),
+                side,
+                start: crate::plugin::TextPosition {
+                    row: stored_index(row, 7)?,
+                    column: stored_index(row, 8)?,
+                },
+                end: crate::plugin::TextPosition {
+                    row: stored_index(row, 9)?,
+                    column: stored_index(row, 10)?,
+                },
+                content: row.get(11).map_err(database_error)?,
+            },
+        });
+    }
+    review.validate()?;
     review.storage_id = Some(id);
     review.storage_revision = revision;
     Ok(review)

src/review_cli.rs

@@ -12,6 +12,7 @@ use vamp::{
 
 pub struct Request {
     id: Option<i64>,
+    delete_comment: Option<u64>,
     repo: Option<PathBuf>,
     json: bool,
     all: bool,
@@ -19,22 +20,37 @@ pub struct Request {
 
 impl Request {
     pub fn parse(args: &mut impl Iterator<Item = OsString>) -> Result<Self, String> {
-        let command = args.next().ok_or("--review requires list or show ID")?;
+        let usage = "--review requires list, show ID, or delete REVIEW_ID COMMENT_ID";
+        let command = args.next().ok_or(usage)?;
         let id = if command == "list" {
             None
-        } else if command == "show" {
+        } else if command == "show" || command == "delete" {
-            let id = args.next().ok_or("show requires a positive review ID")?;
+            let error = "show/delete requires a positive review ID";
+            let id = args.next().ok_or(error)?;
             Some(
                 id.to_str()
                     .and_then(|id| id.parse::<i64>().ok())
                     .filter(|id| *id > 0)
-                    .ok_or("show requires a positive review ID")?,
+                    .ok_or(error)?,
             )
         } else {
-            return Err("--review requires list or show ID".into());
+            return Err(usage.into());
+        };
+        let delete_comment = if command == "delete" {
+            let error = "delete requires a positive comment ID";
+            let id = args.next().ok_or(error)?;
+            Some(
+                id.to_str()
+                    .and_then(|id| id.parse::<u64>().ok())
+                    .filter(|id| *id > 0)
+                    .ok_or(error)?,
+            )
+        } else {
+            None
         };
         Ok(Self {
             id,
+            delete_comment,
             repo: None,
             json: false,
             all: false,
@@ -50,7 +66,8 @@ impl Request {
             self.repo = Some(PathBuf::from(args.next().ok_or("--repo requires a path")?));
         } else if arg == "--json" && !self.json {
             self.json = true;
-        } else if arg == "--all" && self.id.is_some() && !self.all {
+        } else if arg == "--all" && self.id.is_some() && self.delete_comment.is_none() && !self.all
+        {
             self.all = true;
         } else {
             return Err(format!(
@@ -115,10 +132,19 @@ pub fn run(request: &Request, config: Option<&Path>) -> Result<(), String> {
     let root = repository::discover_root(request.repo.as_deref().unwrap_or(&cwd))?;
     let store = Store::open_read_only(&directory)?;
     let output = if let Some(id) = request.id {
-        let review = store
+        let mut review = store
             .as_ref()
             .ok_or("Review does not exist in this repository")?
             .load(&root, id)?;
+        if let Some(comment_id) = request.delete_comment {
+            let index = review
+                .comments
+                .iter()
+                .position(|comment| comment.id == comment_id)
+                .ok_or_else(|| format!("Comment {comment_id} does not exist in review {id}"))?;
+            review.comments.remove(index);
+            Store::open(&directory)?.save(&mut review)?;
+        }
         envelope(&review, request.all, true)
     } else {
         let reviews = match store {

tests/review_cli.rs

@@ -0,0 +1,130 @@
+use std::{fs, path::PathBuf, process::Command};
+use vamp::{
+    plugin::TextPosition,
+    review::{Anchor, Comment, Review, Side, Store, Verdict},
+};
+
+struct Directory(PathBuf);
+impl Drop for Directory {
+    fn drop(&mut self) {
+        let _ = fs::remove_dir_all(&self.0);
+    }
+}
+
+#[test]
+fn cli_deletes_only_the_requested_comment_and_preserves_failed_targets() {
+    let directory = Directory(
+        std::env::temp_dir().join(format!("vamp-review-cli-delete-{}", std::process::id())),
+    );
+    fs::create_dir(&directory.0).unwrap();
+    let root = directory.0.canonicalize().unwrap();
+    for name in ["repo", "other"] {
+        let output = Command::new("jj")
+            .args(["git", "init"])
+            .arg(root.join(name))
+            .env("JJ_CONFIG", "/dev/null")
+            .output()
+            .unwrap();
+        assert!(output.status.success(), "{:?}", output);
+    }
+    let config = root.join("config.rb");
+    fs::write(
+        &config,
+        "Vamp.configure { |config| config.review_directory = 'reviews' }",
+    )
+    .unwrap();
+    let storage = root.join("reviews");
+    let mut store = Store::open(&storage).unwrap();
+    let mut review = Review::new(root.join("repo"));
+    review.summary = "Keep summary".into();
+    review.verdict = Some(Verdict::ChangesRequested);
+    review.comments = [1, u64::MAX]
+        .into_iter()
+        .map(|id| Comment {
+            id,
+            body: format!("Feedback {id}"),
+            resolved: id == u64::MAX,
+            anchor: Anchor {
+                revision: "snapshot".into(),
+                path: "file.rs".into(),
+                side: Side::After,
+                start: TextPosition { row: 0, column: 0 },
+                end: TextPosition { row: 0, column: 1 },
+                content: "界\n".into(),
+            },
+        })
+        .collect();
+    store.save(&mut review).unwrap();
+    let mut other = Review::new(root.join("other"));
+    other.comments = review.comments.clone();
+    store.save(&mut other).unwrap();
+    let id = review.storage_id.unwrap();
+    let cli = |repo: &str, args: &[&str]| {
+        Command::new(env!("CARGO_BIN_EXE_vamp"))
+            .arg("--config")
+            .arg(&config)
+            .arg("--review")
+            .args(args)
+            .current_dir(root.join(repo))
+            .output()
+            .unwrap()
+    };
+    let id_text = id.to_string();
+    for (repo, args) in [
+        ("repo", vec!["delete", "999", "1"]),
+        ("repo", vec!["delete", &id_text, "2"]),
+        ("other", vec!["delete", &id_text, "1"]),
+    ] {
+        let output = cli(repo, &args);
+        assert!(!output.status.success(), "{:?}", output);
+        let loaded = store.load(&review.root, id).unwrap();
+        assert_eq!(loaded.storage_revision, review.storage_revision);
+        assert_eq!(loaded.comments, review.comments);
+    }
+    let output = cli(
+        "repo",
+        &["delete", &id_text, "18446744073709551615", "--json"],
+    );
+    assert!(output.status.success(), "{:?}", output);
+    let json: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap();
+    assert_eq!(json["id"], id);
+    assert_eq!(
+        json["counts"],
+        serde_json::json!({"total": 1, "resolved": 0, "unresolved": 1})
+    );
+    let loaded = store.load(&review.root, id).unwrap();
+    assert_eq!(loaded.storage_revision, review.storage_revision + 1);
+    assert_eq!(loaded.comments, review.comments[..1]);
+    assert_eq!(loaded.summary, review.summary);
+    assert_eq!(loaded.verdict, review.verdict);
+    assert!(store.save(&mut review).is_err()); // Stale editors cannot resurrect the deletion.
+    let output = cli("repo", &["delete", &id_text, "18446744073709551615"]);
+    assert!(!output.status.success());
+    assert_eq!(
+        store.load(&review.root, id).unwrap().storage_revision,
+        loaded.storage_revision
+    );
+    let output = cli("repo", &["delete", &id_text, "1"]);
+    assert!(output.status.success(), "{:?}", output);
+    assert!(
+        String::from_utf8(output.stdout)
+            .unwrap()
+            .contains("0 total")
+    );
+    let empty = store.load(&review.root, id).unwrap();
+    assert!(empty.comments.is_empty());
+    assert_eq!(empty.storage_revision, loaded.storage_revision + 1);
+    assert_eq!(empty.summary, review.summary);
+    assert_eq!(
+        store
+            .load(&other.root, other.storage_id.unwrap())
+            .unwrap()
+            .comments,
+        other.comments
+    );
+    drop(store);
+    fs::remove_dir_all(&storage).unwrap();
+    let output = cli("repo", &["delete", &id_text, "1"]);
+    assert!(!output.status.success());
+    assert!(!storage.exists());
+}

tests/unit/cli_args.rs

@@ -28,12 +28,38 @@ fn review_commands_preserve_filenames_and_validate_arguments() {
             .review
             .is_some()
     );
+    assert!(
+        parse(&[
+            "--review",
+            "delete",
+            "1",
+            "2",
+            "--repo",
+            ".",
+            "--json",
+            "--no-config"
+        ])
+        .unwrap()
+        .unwrap()
+        .review
+        .is_some()
+    );
     for args in [
         vec!["--review"],
         vec!["--review", "show"],
         vec!["--review", "show", "0"],
         vec!["--review", "show", "-1"],
         vec!["--review", "show", "abc"],
+        vec!["--review", "delete"],
+        vec!["--review", "delete", "1"],
+        vec!["--review", "delete", "0", "1"],
+        vec!["--review", "delete", "9223372036854775808", "1"],
+        vec!["--review", "delete", "1", "0"],
+        vec!["--review", "delete", "1", "-1"],
+        vec!["--review", "delete", "1", "abc"],
+        vec!["--review", "delete", "1", "18446744073709551616"],
+        vec!["--review", "delete", "1", "2", "--all"],
+        vec!["--review", "delete", "1", "2", "3"],
         vec!["--review", "list", "--all"],
         vec!["--review", "list", "--repo"],
         vec!["--review", "list", "--repo", ".", "--repo", "."],

tests/unit/review.rs

@@ -151,6 +151,10 @@ fn incompatible_database_schemas_are_preserved() {
     for schema in [
         "CREATE TABLE other (value TEXT); PRAGMA user_version = 42",
         "CREATE TABLE other (value TEXT); PRAGMA user_version = 1",
+        "CREATE TABLE reviews (id INTEGER PRIMARY KEY, root TEXT NOT NULL,
+            revision INTEGER NOT NULL, document TEXT NOT NULL) STRICT;
+         INSERT INTO reviews VALUES (1, '/old/repo', 1, '{\"version\":3}');
+         PRAGMA user_version = 1",
         "",
     ] {
         let directory = directory();
@@ -165,44 +169,73 @@ fn incompatible_database_schemas_are_preserved() {
 }
 
 #[test]
-fn malformed_or_mismatched_database_documents_are_rejected() {
+fn malformed_database_rows_are_rejected_before_loading_snapshots() {
-    let other = directory();
     let directory = directory();
     let root = directory.0.canonicalize().unwrap();
     let mut store = Store::open(&root.join("storage")).unwrap();
     let mut review = Review::new(root.clone());
+    review.comments.push(Comment {
+        id: 1,
+        body: "Feedback".into(),
+        resolved: false,
+        anchor: Anchor {
+            revision: "snapshot".into(),
+            path: "file.rs".into(),
+            side: Side::After,
+            start: crate::plugin::TextPosition { row: 0, column: 0 },
+            end: crate::plugin::TextPosition { row: 0, column: 1 },
+            content: "界\n".into(),
+        },
+    });
     store.save(&mut review).unwrap();
     let id = review.storage_id.unwrap();
-    let mut wrong_root = serde_json::to_value(&review).unwrap();
+    for update in [
-    wrong_root["root"] = serde_json::json!(other.0.canonicalize().unwrap());
+        "resolved = 2",
-    let mut invalid_version = serde_json::to_value(&review).unwrap();
+        "side = 'invalid'",
-    invalid_version["version"] = 1.into();
+        "id = '0'",
-    let mut inherited_identity = serde_json::to_value(&review).unwrap();
+        "id = '18446744073709551616'",
-    inherited_identity["storage_id"] = id.into();
+        "start_row = -1",
-    for document in [
+        "end_column = -1",
-        "{".to_owned(),
-        serde_json::to_string(&wrong_root).unwrap(),
-        serde_json::to_string(&invalid_version).unwrap(),
-        serde_json::to_string(&inherited_identity).unwrap(),
     ] {
+        assert!(
             store
                 .connection
                 .execute(
-                "UPDATE reviews SET document = ?1 WHERE id = ?2",
+                    &format!("UPDATE comments SET {update} WHERE review_id = ?1"),
-                params![document, id],
+                    [id],
                 )
-            .unwrap();
+                .is_err()
-        assert!(store.load(&root, id).is_err());
+        );
-        assert!(store.list(&root).is_err());
+        assert_eq!(store.load(&root, id).unwrap().comments, review.comments);
     }
     store
         .connection
         .execute_batch("PRAGMA ignore_check_constraints = ON")
         .unwrap();
+    for update in [
+        "path = '../outside'",
+        "end_column = 2",
+        "resolved = 2",
+        "side = 'invalid'",
+        "anchor_revision = ''",
+        "id = '0'",
+        "position = 1",
+    ] {
         store
             .connection
             .execute(
-            "UPDATE reviews SET document = CAST(zeroblob(?1) AS TEXT) WHERE id = ?2",
+                &format!("UPDATE comments SET {update} WHERE review_id = ?1"),
+                [id],
+            )
+            .unwrap();
+        assert!(store.load(&root, id).is_err(), "{update}");
+        assert!(store.list(&root).is_err(), "{update}");
+        store.save(&mut review).unwrap();
+    }
+    store
+        .connection
+        .execute(
+            "UPDATE comments SET content = CAST(zeroblob(?1) AS TEXT) WHERE review_id = ?2",
             params![LIMIT as i64 + 1, id],
         )
         .unwrap();
@@ -406,43 +439,82 @@ fn persistence_validates_anchors_and_preserves_files_on_failure() {
 }
 
 #[test]
-fn stored_version_two_is_rejected_without_rewriting_it() {
+fn comment_rows_preserve_order_full_ids_and_atomic_replacement() {
     let directory = directory();
     let root = directory.0.canonicalize().unwrap();
     let mut store = Store::open(&root.join("storage")).unwrap();
     let mut review = Review::new(root.clone());
+    review.comments = [u64::MAX, 7]
+        .into_iter()
+        .map(|id| Comment {
+            id,
+            body: format!("Feedback {id}"),
+            resolved: id == 7,
+            anchor: Anchor {
+                revision: "snapshot".into(),
+                path: "source.rs".into(),
+                side: Side::Before,
+                start: crate::plugin::TextPosition { row: 0, column: 0 },
+                end: crate::plugin::TextPosition { row: 0, column: 1 },
+                content: "界\n".into(),
+            },
+        })
+        .collect();
+    review.summary = "Original".into();
     store.save(&mut review).unwrap();
-    let mut old = serde_json::to_value(&review).unwrap();
+    let id = review.storage_id.unwrap();
-    old["version"] = 2.into();
+    assert_eq!(store.load(&root, id).unwrap().comments, review.comments);
-    let document = serde_json::to_string(&old).unwrap();
+    assert!(
         store
             .connection
             .execute(
-            "UPDATE reviews SET document = ?1 WHERE id = ?2",
+                "UPDATE comments SET review_id = ?1 WHERE review_id = ?2",
-            params![document, review.storage_id.unwrap()],
+                params![id + 1, id],
             )
-        .unwrap();
+            .is_err()
-    assert!(
-        store
-            .load(&root, review.storage_id.unwrap())
-            .unwrap_err()
-            .contains("Unsupported review version 2")
     );
     assert!(
         store
-            .list(&root)
-            .unwrap_err()
-            .contains("Unsupported review version 2")
-    );
-    let preserved: String = store
             .connection
-        .query_row(
+            .execute("UPDATE comments SET id = '7' WHERE review_id = ?1", [id],)
-            "SELECT document FROM reviews WHERE id = ?1",
+            .is_err()
-            [review.storage_id.unwrap()],
+    );
-            |row| row.get(0),
+    store
+        .connection
+        .execute_batch(
+            "CREATE TEMP TRIGGER reject_comment BEFORE INSERT ON comments
+         WHEN NEW.id = '7' BEGIN SELECT RAISE(ABORT, 'blocked comment'); END",
         )
         .unwrap();
-    assert_eq!(preserved, document);
+    let revision = review.storage_revision;
+    let original = review.comments.clone();
+    review.summary = "Unsaved".into();
+    review.comments[0].body = "Changed".into();
+    assert!(
+        store
+            .save(&mut review)
+            .unwrap_err()
+            .contains("blocked comment")
+    );
+    assert_eq!(review.storage_revision, revision);
+    let loaded = store.load(&root, id).unwrap();
+    assert_eq!(loaded.summary, "Original");
+    assert_eq!(loaded.comments, original);
+    assert_eq!(loaded.storage_revision, revision);
+    store
+        .connection
+        .execute_batch("DROP TRIGGER reject_comment")
+        .unwrap();
+    review.comments.reverse();
+    store.save(&mut review).unwrap();
+    assert_eq!(store.load(&root, id).unwrap().comments, review.comments);
+    review.comments.pop();
+    store.save(&mut review).unwrap();
+    assert_eq!(store.load(&root, id).unwrap().comments, review.comments);
+    review.comments.clear();
+    store.save(&mut review).unwrap();
+    assert!(store.load(&root, id).unwrap().comments.is_empty());
+    assert_eq!(store.list(&root).unwrap()[0].summary, "Unsaved");
 }
 
 #[test]