fix: hide commentless reviews from CLI listings
This commit is contained in:
@@ -34,6 +34,8 @@ Use the persistent **Review ID** printed by `list`, not a list position. IDs are
|
||||
|
||||
`show` includes unresolved comments by default; `--all` includes resolved comments too. Human-readable output is the default, with source excerpts in language-tagged Markdown code blocks. 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 raw selected source excerpts—not whole source files or Markdown fences. `list --json` returns the repository and review metadata/counts without comments. Explicit editor JSON exports still contain complete anchor source text.
|
||||
|
||||
`list` omits reviews with no comments in both text and JSON output. Reviews containing only resolved comments remain listed. Commentless reviews remain stored to preserve file checkmarks, summaries, and verdicts; the editor can still resume them, and `show ID` can inspect them directly.
|
||||
|
||||
`list` and `show` are read-only. `delete REVIEW_ID COMMENT_ID` immediately commits deletion of one resolved or unresolved comment. `resolve REVIEW_ID COMMENT_ID` instead marks that comment resolved, preserving its body and original anchor. Repeating resolution succeeds without writing or advancing the review revision. Both commands print the updated review with its remaining unresolved comments (`--json` is supported); use `show REVIEW_ID --all` to verify resolved feedback. 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 change. Existing JSON exports are not modified.
|
||||
|
||||
`vamp review` still opens a file named `review`; `vamp -- --review` opens a file named `--review`.
|
||||
|
||||
@@ -20,6 +20,7 @@ One editor crate; the workspace also includes its patched Crossterm dependency.
|
||||
- `review`: owns the validated local review model, exact immutable anchors, persisted file checkmarks, SQLite storage through bundled rusqlite, and atomic JSON import/export with external-change protection. Schema 4 stores review identity, repository, revision, summary, verdict, and nullable ordered comparison endpoints in `reviews`; null endpoints identify source reviews. `comments` stores ordered comment IDs, bodies, resolution states, and explicit immutable anchor fields. `reviewed_files` stores revision pairs, paths, optional old paths, and optional exact live After text. 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` retains compare-and-swap review revisions and atomic whole-review saves: metadata and replacement comment/checkmark rows commit together. A stored review cannot be reassigned to another scope. 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/checkmark overhead. JSON version 5 is an independent import/export format. Incompatible databases, including schema 3, are rejected and preserved without migration or reset. CLI metadata and selectors reuse loaded reviews; selector choices live only in the displayed buffer.
|
||||
The review CLI keeps `list`/`show` read-only. Comment deletion and resolution validate 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. Resolution preserves comment bodies and immutable anchors; already-resolved comments succeed without writing or advancing the revision.
|
||||
CLI listings exclude reviews with zero comments, not reviews with zero unresolved comments. Filtering stays at the CLI boundary: storage listing and editor restoration retain commentless reviews and their file checkmarks, summaries, and verdicts.
|
||||
`editor_review` owns tab-local review state selected by canonical repository root and ordered resolved comparison identities, or a separate source scope for ordinary repository buffers. A sole scope-matching review restores automatically; multiple matches open the floating selector. `diff` freezes initial review endpoints through in-place Working refreshes; explicit comparisons and moved bookmark endpoints resolve their own scope. Source and comparison panes share the comment editor, immutable anchors, storage, and review commands, not their review documents. Dirty drafts block scope changes, and stored/JSON opens reject mismatched scopes before replacing state. Comment/summary text saves explicitly; dismissing discards that editor's unsaved text, while switching may retain hidden drafts. Verdicts/deletions/resolution changes commit immediately without saving drafts. 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`. 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.
|
||||
|
||||
+5
-1
@@ -167,7 +167,11 @@ pub fn run(request: &Request, config: Option<&Path>) -> Result<(), String> {
|
||||
vec![review]
|
||||
} else {
|
||||
match store {
|
||||
Some(store) => store.list(&root)?,
|
||||
Some(store) => store
|
||||
.list(&root)?
|
||||
.into_iter()
|
||||
.filter(|review| !review.comments.is_empty())
|
||||
.collect(),
|
||||
None => Vec::new(),
|
||||
}
|
||||
};
|
||||
|
||||
+41
-8
@@ -115,6 +115,17 @@ fn cli_deletes_only_the_requested_comment_and_preserves_failed_targets() {
|
||||
assert!(empty.comments.is_empty());
|
||||
assert_eq!(empty.storage_revision, loaded.storage_revision + 1);
|
||||
assert_eq!(empty.summary, review.summary);
|
||||
let output = cli("repo", &["list", "--json"]);
|
||||
assert!(output.status.success(), "{:?}", output);
|
||||
let list: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap();
|
||||
assert_eq!(list["reviews"], serde_json::json!([]));
|
||||
let output = cli("repo", &["list"]);
|
||||
assert!(output.status.success(), "{:?}", output);
|
||||
assert!(
|
||||
String::from_utf8(output.stdout)
|
||||
.unwrap()
|
||||
.contains("No stored reviews.")
|
||||
);
|
||||
assert_eq!(
|
||||
store
|
||||
.load(&other.root, other.storage_id.unwrap())
|
||||
@@ -178,6 +189,8 @@ fn cli_resolves_only_the_requested_comment_and_preserves_original_feedback() {
|
||||
other.comments = review.comments.clone();
|
||||
store.save(&mut other).unwrap();
|
||||
let mut source = Review::new(review.root.clone());
|
||||
source.summary = "Progress without comments".into();
|
||||
source.verdict = Some(Verdict::LooksGood);
|
||||
store.save(&mut source).unwrap();
|
||||
let mut reversed = Review::new(review.root.clone());
|
||||
reversed.comparison = Some(["working:frozen-tip".into(), "frozen-base".into()]);
|
||||
@@ -272,20 +285,32 @@ fn cli_resolves_only_the_requested_comment_and_preserves_original_feedback() {
|
||||
let output = cli("repo", &["list", "--json"]);
|
||||
assert!(output.status.success(), "{:?}", output);
|
||||
let list: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap();
|
||||
assert_eq!(list["reviews"].as_array().unwrap().len(), 3);
|
||||
assert_eq!(
|
||||
list["reviews"][0]["comparison"],
|
||||
serde_json::json!(reversed.comparison)
|
||||
list["reviews"]
|
||||
.as_array()
|
||||
.unwrap()
|
||||
.iter()
|
||||
.map(|review| review["id"].as_i64().unwrap())
|
||||
.collect::<Vec<_>>(),
|
||||
vec![id]
|
||||
);
|
||||
assert!(list["reviews"][1]["comparison"].is_null());
|
||||
assert_eq!(list["reviews"][2]["comparison"], json["comparison"]);
|
||||
assert_eq!(list["reviews"][2]["counts"], json["counts"]);
|
||||
assert_eq!(list["reviews"][0]["comparison"], json["comparison"]);
|
||||
assert_eq!(list["reviews"][0]["counts"], json["counts"]);
|
||||
let output = cli("repo", &["list"]);
|
||||
assert!(output.status.success(), "{:?}", output);
|
||||
let text = String::from_utf8(output.stdout).unwrap();
|
||||
assert!(text.contains("Scope: source"));
|
||||
assert!(!text.contains("Progress without comments"));
|
||||
assert!(text.contains("Scope: comparison frozen-base → working:frozen-tip"));
|
||||
assert!(text.contains("Scope: comparison working:frozen-tip → frozen-base"));
|
||||
assert!(!text.contains("Scope: comparison working:frozen-tip → frozen-base"));
|
||||
assert_eq!(fs::read(&path).unwrap(), before);
|
||||
let output = cli(
|
||||
"repo",
|
||||
&["show", &source.storage_id.unwrap().to_string(), "--json"],
|
||||
);
|
||||
assert!(output.status.success(), "{:?}", output);
|
||||
let shown: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap();
|
||||
assert_eq!(shown["summary"], source.summary);
|
||||
assert_eq!(shown["verdict"], "looks_good");
|
||||
let output = cli("repo", &["resolve", &id_text, "2"]);
|
||||
assert!(output.status.success(), "{:?}", output);
|
||||
let text = String::from_utf8(output.stdout).unwrap();
|
||||
@@ -293,6 +318,14 @@ fn cli_resolves_only_the_requested_comment_and_preserves_original_feedback() {
|
||||
let loaded = store.load(&review.root, id).unwrap();
|
||||
assert_eq!(loaded.storage_revision, review.storage_revision + 2);
|
||||
assert!(loaded.comments.iter().all(|comment| comment.resolved));
|
||||
let output = cli("repo", &["list", "--json"]);
|
||||
assert!(output.status.success(), "{:?}", output);
|
||||
let list: serde_json::Value = serde_json::from_slice(&output.stdout).unwrap();
|
||||
assert_eq!(list["reviews"][0]["id"], id);
|
||||
assert_eq!(
|
||||
list["reviews"][0]["counts"],
|
||||
serde_json::json!({"total": 3, "resolved": 3, "unresolved": 0})
|
||||
);
|
||||
assert_eq!(
|
||||
store
|
||||
.load(&other.root, other.storage_id.unwrap())
|
||||
|
||||
Reference in New Issue
Block a user