From b9895b74400c5e5ec42a5b50afcd1ad79a872f42 Mon Sep 17 00:00:00 2001 From: yunyaozhou Date: Wed, 9 Sep 2026 17:09:43 +0800 Subject: [PATCH 01/10] feat(review): send pending feedback and archive finished reviews --- Cargo.lock | 12 + README.md | 22 +- crates/plannotator-tui/Cargo.toml | 1 + crates/plannotator-tui/README.md | 12 +- .../plannotator-tui/src/app/archive_view.rs | 103 ++++++++ crates/plannotator-tui/src/app/draw.rs | 39 ++- crates/plannotator-tui/src/app/feedback.rs | 242 ++++++++++++++++++ .../plannotator-tui/src/app/feedback/tests.rs | 188 ++++++++++++++ crates/plannotator-tui/src/app/header.rs | 83 +++++- crates/plannotator-tui/src/app/input.rs | 41 ++- crates/plannotator-tui/src/app/mod.rs | 123 +++------ crates/plannotator-tui/src/app/review.rs | 186 ++++++++++++++ .../plannotator-tui/src/app/review/tests.rs | 240 +++++++++++++++++ .../src/app/review_test_support.rs | 117 +++++++++ crates/plannotator-tui/src/app/send.rs | 178 +++++++------ crates/plannotator-tui/src/store.rs | 63 ++--- crates/plannotator-tui/src/store/review.rs | 148 +++++++++++ .../plannotator-tui/src/store/review/tests.rs | 180 +++++++++++++ 18 files changed, 1736 insertions(+), 242 deletions(-) create mode 100644 crates/plannotator-tui/src/app/archive_view.rs create mode 100644 crates/plannotator-tui/src/app/feedback.rs create mode 100644 crates/plannotator-tui/src/app/feedback/tests.rs create mode 100644 crates/plannotator-tui/src/app/review.rs create mode 100644 crates/plannotator-tui/src/app/review/tests.rs create mode 100644 crates/plannotator-tui/src/app/review_test_support.rs create mode 100644 crates/plannotator-tui/src/store/review.rs create mode 100644 crates/plannotator-tui/src/store/review/tests.rs diff --git a/Cargo.lock b/Cargo.lock index 480c224..c8e329c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1304,6 +1304,7 @@ dependencies = [ "serde", "serde_json", "similar", + "time", "toml", "tui-markdown", "unicode-width", @@ -2038,6 +2039,7 @@ dependencies = [ "powerfmt", "serde_core", "time-core", + "time-macros", ] [[package]] @@ -2046,6 +2048,16 @@ version = "0.1.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9e1c906769ad99c88eaa54e728060edef082f8e358ff32030cb7c7d315e81109" +[[package]] +name = "time-macros" +version = "0.2.32" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7e689342a48d2ea927c87ea50cabf8594854bf940e9310208848d680d668ed85" +dependencies = [ + "num-conv", + "time-core", +] + [[package]] name = "toml" version = "1.1.4+spec-1.1.0" diff --git a/README.md b/README.md index 6e24345..164d614 100644 --- a/README.md +++ b/README.md @@ -50,27 +50,41 @@ plannotator-tui last # your coding agent's recent replies, pick on ``` Drag with the mouse (or `v` and move) to select, then `a` ๐Ÿ‘ ยท `c` ๐Ÿ’ฌ ยท `d` โœ—. `E` copies the -review to the clipboard as numbered annotations (`# Annotations on plan.md`, `## Annotation 1 +feedback to the clipboard as numbered annotations (`# Annotations on plan.md`, `## Annotation 1 (line 12)`, โ€ฆ). Every annotation is saved as JSON the moment you make it; `q` closes. Copies go to the clipboard as OSC 52, which is the terminal you are looking at, so on Herdr 0.9.0 they reach your own machine even when the app runs on a remote server; Herdr Annotate's global `copy-context` and `copy-archive` actions do not, because they run outside a pane. +For file and folder reviews, `E` sends only new or edited annotations. Send A and B, then +add C: the next send includes just C. Sent notes stay visible with a marker; editing one +makes it pending again, including after a restart. `R` **Resend all** includes every active +note, with the button showing how many are already sent. With nothing pending, `E` reports +โ€œnothing new to sendโ€. A failed send keeps the notes pending for retry. + +`F` **Finish review** archives sent, unchanged notes and leaves pending ones in place. +`U` undoes the last finish during this session. `H` opens the archive, where Enter or a +click restores a note even after reopening the app. Restoring keeps its original id and +sent status. The archive is stored with the annotations and works even when feedback +history is turned off. + | Where | Keys | |---|---| | anywhere | `Tab` cycle tree ยท document ยท notes; `E` send; `t` tree; `r` reload; `q` quit | | document | `j`/`k` block; `c` comment on the block; `x` clear its annotations; `v` select with `hjkl` `w` `b` `0` `$` | | toolbar | `a` looks good ยท `c` comment ยท `d` delete ยท `Esc` | | notes | `j`/`k`; `e` edit; `x` remove; click a bubble | -| tree | `j`/`k`; `Enter` open; `E` sends every annotated file | +| file/folder review | `E` send new ยท `R` resend all ยท `F` finish review ยท `U` undo finish ยท `H` archive | +| tree | `j`/`k`; `Enter` open; `E` sends new notes across all reviewed files, including collapsed folders | ## Inside Herdr Install [Herdr Annotate](https://github.com/plannotator/herdr-annotate); it bundles this binary, opens it in a pane with `prefix+o` (folder) or `prefix+shift+o` (agent's last reply) or by Ctrl-clicking a `file://โ€ฆmd` link, and the header button sends the review straight back to -the agent as its next message: `Send 3 to claude in w1:p2 โ–ธ`. +the agent as its next message: `Send 3 new โ–ธ claude in w1:p2 (E)`. Folder reviews show +`Send 3 new across 2 files` and send one combined feedback message. ```toml # ~/.config/plannotator-tui/config.toml @@ -134,7 +148,7 @@ browser app writes, so both tools share one history. To turn it off, set ## Headless ```sh -plannotator-tui --export # the review, to stdout +plannotator-tui --export # all active notes, to stdout (no delivery recorded) plannotator-tui --annotate [comment|looks_good|delete] plannotator-tui --snapshot [cols rows scroll] [quote] # one frame as text plannotator-tui --bench # parse / layout timings diff --git a/crates/plannotator-tui/Cargo.toml b/crates/plannotator-tui/Cargo.toml index 29da54a..85cb8f4 100644 --- a/crates/plannotator-tui/Cargo.toml +++ b/crates/plannotator-tui/Cargo.toml @@ -22,6 +22,7 @@ serde_json.workspace = true similar.workspace = true toml.workspace = true unicode-width.workspace = true +time = { version = "0.3", features = ["formatting", "parsing"] } pulldown-cmark = { version = "0.13.4", default-features = false } ratatui = "0.30" # highlight-code pulls syntect + a C oniguruma build; plain code blocks keep the build pure Rust. diff --git a/crates/plannotator-tui/README.md b/crates/plannotator-tui/README.md index 1edb7a3..c90f11e 100644 --- a/crates/plannotator-tui/README.md +++ b/crates/plannotator-tui/README.md @@ -16,10 +16,18 @@ cargo build --release | document | drag with the mouse, or `v` then `hjkl` / `w` `b` / `0` `$` to select; `Enter` confirms ยท `j`/`k` or click selects a block ยท `c` comments on the block ยท `x` clears the block's annotations | | selection toolbar | `a` ๐Ÿ‘ looks good ยท `c` ๐Ÿ’ฌ comment (opens a box at the selection) ยท `d` โœ— delete ยท `Esc` clears | | rail | `j`/`k` move ยท `e` / `Enter` edit body ยท `x` remove ยท click a bubble to focus it | -| tree | `j`/`k` move ยท `Enter` open ยท `E` send feedback for every annotated file ยท counts show per file | +| file/folder review | `E` send new or edited notes ยท `R` resend all active notes ยท `F` archive sent notes ยท `U` undo finish ยท `H` open archive | +| tree | `j`/`k` move ยท `Enter` open ยท `E` send new feedback across files, including collapsed folders ยท counts show active notes per file | +| archive | `j`/`k` or โ†‘/โ†“ select ยท `Enter` or click restore ยท `Esc` close | Selections and exports are copied to the terminal clipboard (OSC 52). +Sent notes remain visible. File reviews send only new or edited notes by default; +`R` explicitly includes the sent ones too. `F` archives only sent notes that have not +changed since delivery. Archived notes survive restarts, and restoring them keeps their +ids and delivery history. Failed sends keep the feedback pending. These actions also +have clickable buttons, which wrap onto another header row in narrow panes. + ## Where things live Every annotation is saved the moment it is made, as JSON, in the Plannotator data directory: @@ -37,7 +45,7 @@ documents (an agent's last message, stdin) are never persisted. ## Headless tools ```bash -plannotator-tui --export # feedback markdown to stdout +plannotator-tui --export # all active feedback to stdout; does not mark it sent plannotator-tui --bench # parse / render / reflow timings plannotator-tui --blocks # block index, kind, first row plannotator-tui --annotate [comment|looks_good|delete] diff --git a/crates/plannotator-tui/src/app/archive_view.rs b/crates/plannotator-tui/src/app/archive_view.rs new file mode 100644 index 0000000..2b47c7b --- /dev/null +++ b/crates/plannotator-tui/src/app/archive_view.rs @@ -0,0 +1,103 @@ +//! A small archive picker: identify a note by file, quote and body, then restore it. + +use ratatui::Frame; +use ratatui::crossterm::event::{KeyCode, KeyEvent, KeyModifiers, MouseButton, MouseEvent, MouseEventKind}; +use ratatui::layout::Rect; +use ratatui::style::{Color, Style}; +use ratatui::text::{Line, Span}; +use ratatui::widgets::{Block, BorderType, Borders, Clear, Paragraph}; + +use super::{App, Mode, label}; + +impl App { + pub(super) fn archive_key(&mut self, key: KeyEvent) { + match key.code { + KeyCode::Char('j') | KeyCode::Down => self.move_archive_cursor(1), + KeyCode::Char('k') | KeyCode::Up => self.move_archive_cursor(-1), + KeyCode::Enter => self.restore_selected_archived(), + KeyCode::Esc | KeyCode::Char('H' | 'q') => self.mode = Mode::Browse, + KeyCode::Char('c') if key.modifiers == KeyModifiers::CONTROL => self.mode = Mode::Browse, + _ => {} + } + } + + fn move_archive_cursor(&mut self, delta: i64) { + let last = self.archive_items.len().saturating_sub(1); + self.archive_cursor = (self.archive_cursor as i64 + delta).clamp(0, last as i64) as usize; + } + + pub(super) fn archive_mouse(&mut self, mouse: MouseEvent) { + match mouse.kind { + MouseEventKind::ScrollDown => self.move_archive_cursor(1), + MouseEventKind::ScrollUp => self.move_archive_cursor(-1), + MouseEventKind::Down(MouseButton::Left) => { + let hit = self.geometry.archive_rows.iter().find(|(rect, _)| { + mouse.column >= rect.x + && mouse.column < rect.right() + && mouse.row >= rect.y + && mouse.row < rect.bottom() + }); + if let Some((_, index)) = hit { + self.archive_cursor = *index; + self.restore_selected_archived(); + } + } + _ => {} + } + } + + pub(super) fn draw_archive(&mut self, frame: &mut Frame) { + let area = frame.area(); + let width = area.width.saturating_sub(4).clamp(1, 100).min(area.width); + let height = area.height.saturating_sub(4).clamp(3, 22).min(area.height); + let rect = Rect { + x: area.x + (area.width - width) / 2, + y: area.y + (area.height - height) / 2, + width, + height, + }; + frame.render_widget(Clear, rect); + let block = Block::default() + .borders(Borders::ALL) + .border_type(BorderType::Rounded) + .border_style(Style::new().fg(Color::Cyan)) + .title(format!(" Archived annotations ({}) ", self.archive_items.len())) + .title_bottom(" โ†‘โ†“ select ยท enter/click restore ยท esc close "); + let inner = block.inner(rect); + frame.render_widget(block, rect); + if self.archive_items.is_empty() { + frame.render_widget( + Paragraph::new("No archived annotations. F finishes sent annotations."), + inner, + ); + return; + } + let visible = (usize::from(inner.height) / 3).max(1); + let start = self.archive_cursor.saturating_sub(visible - 1); + for (index, item) in self.archive_items.iter().enumerate().skip(start).take(visible) { + let y = inner.y + ((index - start) * 3) as u16; + if y >= inner.bottom() { + break; + } + let path = self.review_file_name(&item.path); + let quote = item.annotation.anchor.original_text.split_whitespace().collect::>().join(" "); + let body = item.annotation.body.split_whitespace().collect::>().join(" "); + let style = if index == self.archive_cursor { + Style::new().fg(Color::Black).bg(Color::Cyan) + } else { + Style::new() + }; + let rows = vec![ + Line::from(Span::styled( + format!(" {path} ยท {}", label(item.annotation.anchor.kind())), + style, + )), + Line::from(Span::styled(format!(" โ€œ{quote}โ€"), style)), + Line::from(Span::styled(format!(" {body}"), style)), + ]; + let row_rect = Rect { x: inner.x, y, width: inner.width, height: 3.min(inner.bottom() - y) }; + frame.render_widget(Paragraph::new(rows).style(style), row_rect); + self.geometry.archive_rows.push((row_rect, index)); + } + } +} diff --git a/crates/plannotator-tui/src/app/draw.rs b/crates/plannotator-tui/src/app/draw.rs index 8f25a31..fff93bb 100644 --- a/crates/plannotator-tui/src/app/draw.rs +++ b/crates/plannotator-tui/src/app/draw.rs @@ -48,8 +48,12 @@ fn priority(kind: Kind) -> u8 { impl App { pub(crate) fn draw(&mut self, frame: &mut Frame) { let area = frame.area(); - let [header, body, footer] = - Layout::vertical([Constraint::Length(1), Constraint::Min(1), Constraint::Length(1)]).areas(area); + let [header, body, footer] = Layout::vertical([ + Constraint::Length(self.header_height(area.width)), + Constraint::Min(1), + Constraint::Length(1), + ]) + .areas(area); let show_tree = self.tree_shown(area.width) || (self.tree.is_some() && self.focus == Focus::Tree); let tree_width = if show_tree { TREE_WIDTH } else { 0 }; @@ -66,14 +70,7 @@ impl App { Constraint::Length(rail_width), ]) .areas(body); - self.geometry = Geometry { - tree, - doc, - toolbar: None, - bubbles: Vec::new(), - send_button: None, - pick_rows: Vec::new(), - }; + self.geometry = Geometry { tree, doc, ..Geometry::default() }; if self.open.layout.width != usize::from(doc.width) { self.open.layout.reflow(usize::from(doc.width)); @@ -97,6 +94,7 @@ impl App { Mode::Edit(_) => self.draw_compose(frame, &self.compose_title("edit")), Mode::Browse if self.pending.is_some() => self.draw_toolbar(frame), Mode::Pick => self.draw_pick(frame), + Mode::Archive => self.draw_archive(frame), Mode::Browse | Mode::ConfirmQuit => {} } } @@ -345,8 +343,13 @@ impl App { let border = if highlighted { Style::new().fg(accent(kind)) } else { Style::new().fg(Color::DarkGray) }; let border = if rail_focused && index == self.rail_cursor { border.bold() } else { border }; + let sent = if self.is_file_review() && !self.open.store.is_pending(placed.annotation) { + " ยท sent" + } else { + "" + }; let title = Span::styled( - format!(" {} {} ", glyph(kind), short_id(&placed.annotation.id)), + format!(" {} {}{sent} ", glyph(kind), short_id(&placed.annotation.id)), Style::new().fg(accent(kind)), ); let bubble = Block::default() @@ -367,7 +370,7 @@ impl App { self.geometry.bubbles = bubbles; } - fn draw_footer(&self, frame: &mut Frame, area: Rect) { + fn draw_footer(&mut self, frame: &mut Frame, mut area: Rect) { if self.mode == Mode::ConfirmQuit { // The question owns the footer: the browse help would name keys that are not // live while it is up. @@ -378,6 +381,18 @@ impl App { frame.render_widget(Paragraph::new(Line::from(Span::raw(question).bold())), area); return; } + if self.mode == Mode::Browse && !self.undo_archive.is_empty() { + let label = " U Undo finish "; + let width = (label.width() as u16).min(area.width); + let rect = Rect { x: area.right() - width, y: area.y, width, height: area.height }; + frame.render_widget(Paragraph::new(label).style(Style::new().fg(Color::Cyan)), rect); + self.geometry.undo_button = Some(rect); + area.width = area.width.saturating_sub(width); + } + if let Some(status) = &self.status { + frame.render_widget(Paragraph::new(format!(" {status}")), area); + return; + } let orphans = self.open.store.orphans(); // The status leads: it is the transient half of the line, and the name and counters // it pushes right are on screen for the whole session anyway. diff --git a/crates/plannotator-tui/src/app/feedback.rs b/crates/plannotator-tui/src/app/feedback.rs new file mode 100644 index 0000000..0fa0454 --- /dev/null +++ b/crates/plannotator-tui/src/app/feedback.rs @@ -0,0 +1,242 @@ +//! Select feedback once: the body, count, history and delivery ids all cover that set. +//! Folder counts are cached on open/change/reload, never read from disk while drawing. + +use std::collections::HashMap; +use std::path::{Path, PathBuf}; + +use anyhow::{Context, Result}; +use plannotator_tui_schema::{Kind, Provenance}; + +use super::App; +use crate::archive::AnnotationRecord; +use crate::doc::Document; +use crate::export; +use crate::store::{Location, Store}; + +#[cfg(test)] +mod tests; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(super) enum SendScope { + Pending, + All, +} + +#[derive(Debug, Default, Clone, Copy)] +pub(super) struct ReviewCounts { + pub(super) pending: usize, + pub(super) sent: usize, + pub(super) archived: usize, +} + +impl ReviewCounts { + pub(super) fn for_store(store: &Store) -> Self { + let placed = store.placed(); + let pending = placed.iter().filter(|p| store.is_pending(p.annotation)).count(); + Self { pending, sent: placed.len() - pending, archived: store.archived().len() } + } +} + +#[derive(Debug)] +pub(super) struct FeedbackPart { + pub(super) path: Option, + pub(super) store: Store, + pub(super) ids: Vec, +} + +#[derive(Debug, Default)] +pub(super) struct Feedback { + pub(super) text: String, + pub(super) count: usize, + pub(super) parts: Vec, + pub(super) annotations: Vec, + pub(super) counts: HashMap, +} + +impl Feedback { + fn add(&mut self, path: Option, name: &str, doc: &Document, store: Store, scope: SendScope) { + if let Some(path) = &path { + self.counts.insert(path.clone(), ReviewCounts::for_store(&store)); + } + let entries: Vec> = store + .placed() + .into_iter() + .filter(|p| scope == SendScope::All || store.is_pending(p.annotation)) + .map(|p| export::Entry { + annotation: p.annotation, + lines: export::line_span(&doc.source, p.range), + range: p.range.clone(), + }) + .collect(); + if entries.is_empty() { + return; + } + if !self.text.is_empty() { + self.text.push('\n'); + } + self.text.push_str(&export::feedback(&doc.source, name, &entries)); + let ids = entries.iter().map(|e| e.annotation.id.clone()).collect(); + self.count += entries.len(); + self.annotations.extend(entries.iter().map(|entry| { + let a = entry.annotation; + AnnotationRecord { + id: Some(a.id.clone()), + kind: Some( + match a.anchor.kind() { + Kind::Comment => "comment", + Kind::LooksGood => "looks-good", + Kind::Delete => "delete", + } + .to_owned(), + ), + text: (!a.body.is_empty()).then(|| a.body.clone()), + original_text: (!a.anchor.original_text.is_empty()).then(|| a.anchor.original_text.clone()), + } + })); + self.parts.push(FeedbackPart { path, store, ids }); + } + + fn exported_text(self) -> String { + if self.text.is_empty() { "No annotations.".to_owned() } else { self.text } + } +} + +impl App { + pub(super) fn is_file_review(&self) -> bool { + self.tree.is_some() + || (!self.open.source.transient && matches!(self.open.source.provenance, Provenance::File { .. })) + } + + pub(super) fn review_counts(&self) -> ReviewCounts { + if self.tree.is_none() { + return ReviewCounts::for_store(&self.open.store); + } + self.folder_counts.values().fold(ReviewCounts::default(), |mut sum, count| { + sum.pending += count.pending; + sum.sent += count.sent; + sum.archived += count.archived; + sum + }) + } + + pub(super) fn pending_file_count(&self) -> usize { + self.folder_counts.values().filter(|c| c.pending > 0).count() + } + + /// Both active and archive-only records belong to the review. Restrict project-wide + /// records to this folder; a sibling in the same git project must not be sent. + pub(super) fn review_files(&self) -> Vec { + let Some(tree) = &self.tree else { + return match &self.open.source.provenance { + Provenance::File { path } => vec![path.clone()], + _ => Vec::new(), + }; + }; + let mut paths = Store::annotated_documents(&self.data_dir, &self.project); + paths.extend(tree.rows.iter().filter(|r| !r.is_dir && r.annotations > 0).map(|r| r.path.clone())); + if let Provenance::File { path } = &self.open.source.provenance { + paths.push(path.clone()); + } + paths.sort(); + paths.dedup(); + paths.retain(|p| p.starts_with(tree.root())); + paths + } + + pub(super) fn review_file_name(&self, path: &Path) -> String { + if let Some(tree) = &self.tree { + path.strip_prefix(tree.root()).unwrap_or(path).display().to_string() + } else { + path.file_name() + .map_or_else(|| path.display().to_string(), |name| name.to_string_lossy().into_owned()) + } + } + + pub(super) fn is_open(&self, path: &Path) -> bool { + matches!(&self.open.source.provenance, Provenance::File { path: p } if p == path) + } + + /// Other files need source and anchor resolution, not a rendered document layout. + pub(super) fn load_review_file(&self, path: &Path) -> Result<(Document, Store)> { + let content = match std::fs::read_to_string(path) { + Ok(content) => content, + // A deleted file's finished notes remain recoverable. Restoring them makes + // them orphans until their source is available again. + Err(err) if err.kind() == std::io::ErrorKind::NotFound => String::new(), + Err(err) => return Err(err).with_context(|| format!("reading {}", path.display())), + }; + let doc = Document::parse(content); + let store = Store::load(&Location::for_file(&self.data_dir, &self.project, path), &doc)?; + Ok((doc, store)) + } + + pub(super) fn refresh_review_counts(&mut self) -> Result<()> { + if self.tree.is_none() { + return Ok(()); + } + let mut counts = HashMap::new(); + for path in self.review_files() { + let count = if self.is_open(&path) && path.is_file() { + ReviewCounts::for_store(&self.open.store) + } else { + let (_, store) = self.load_review_file(&path)?; + ReviewCounts::for_store(&store) + }; + counts.insert(path, count); + } + self.folder_counts = counts; + Ok(()) + } + + pub(super) fn update_open_review_counts(&mut self) { + if self.tree.is_some() + && let Provenance::File { path } = &self.open.source.provenance + { + let mut counts = ReviewCounts::for_store(&self.open.store); + if !path.is_file() { + counts.pending = 0; + counts.sent = 0; + } + self.folder_counts.insert(path.clone(), counts); + } + } + + fn file_feedback(&self, scope: SendScope) -> Feedback { + let path = match &self.open.source.provenance { + Provenance::File { path } => Some(path.clone()), + _ => None, + }; + let mut feedback = Feedback::default(); + feedback.add(path, &self.open.source.name, &self.open.doc, self.open.store.clone(), scope); + feedback + } + + pub(super) fn prepare_feedback(&self, scope: SendScope) -> Result { + let Some(tree) = &self.tree else { return Ok(self.file_feedback(scope)) }; + let mut feedback = Feedback::default(); + for path in self.review_files() { + if !path.is_file() { + let (_, store) = self.load_review_file(&path)?; + feedback.counts.insert(path, ReviewCounts::for_store(&store)); + continue; + } + let name = path.strip_prefix(tree.root()).unwrap_or(&path).display().to_string(); + if self.is_open(&path) { + feedback.add(Some(path), &name, &self.open.doc, self.open.store.clone(), scope); + } else { + let (doc, store) = self.load_review_file(&path)?; + feedback.add(Some(path), &name, &doc, store, scope); + } + } + Ok(feedback) + } + + /// Headless export retains the full active review and does not record a delivery. + pub(crate) fn feedback(&self) -> String { + self.file_feedback(SendScope::All).exported_text() + } + + pub(crate) fn folder_feedback(&self) -> Result { + Ok(self.prepare_feedback(SendScope::All)?.exported_text()) + } +} diff --git a/crates/plannotator-tui/src/app/feedback/tests.rs b/crates/plannotator-tui/src/app/feedback/tests.rs new file mode 100644 index 0000000..a048d9b --- /dev/null +++ b/crates/plannotator-tui/src/app/feedback/tests.rs @@ -0,0 +1,188 @@ +#![allow(clippy::expect_used, clippy::indexing_slicing, reason = "tests assert by panicking")] + +use plannotator_tui_schema::{DocumentSource, Kind, Provenance}; + +use crate::app::review_test_support::{Outcome, click, draw, file_app, folder_app, press, reopen}; +use crate::app::{Mode, Open}; +use crate::store::Location; + +#[test] +fn incremental_send_and_explicit_resend_use_the_same_set_for_body_history_and_ids() { + let (root, mut app, delivery) = file_app("incremental"); + app.add_quote_annotation("one", Kind::Comment, "note A".into()).expect("A"); + app.add_quote_annotation("two", Kind::Comment, "note B".into()).expect("B"); + let b = app.open.store.placed()[1].annotation.id.clone(); + press(&mut app, 'E'); + app.add_quote_annotation("three", Kind::Comment, "note C".into()).expect("C"); + assert_eq!(app.send_count(), 1); + draw(&mut app, 80, 24); + let send = app.geometry.send_button.expect("send button"); + click(&mut app, send); + app.open.store.edit_body(&b, "note B revised".into()).expect("edit B"); + reopen(&mut app); + assert_eq!(app.send_count(), 1, "edited B remains pending after reopening"); + press(&mut app, 'E'); + assert_eq!(app.send_count(), 0); + let screen = draw(&mut app, 80, 24); + assert!(screen.contains("Resend all (3 sent)"), "{screen}"); + let resend = app.geometry.resend_button.expect("resend button"); + click(&mut app, resend); + + let calls = delivery.calls.borrow(); + assert_eq!(calls.len(), 4); + assert!(calls[0].contains("note A") && calls[0].contains("note B")); + assert!(calls[1].contains("note C") && !calls[1].contains("note A") && !calls[1].contains("note B")); + assert!( + calls[2].contains("note B revised") && !calls[2].contains("note A") && !calls[2].contains("note C") + ); + assert!( + calls[3].contains("note A") && calls[3].contains("note B revised") && calls[3].contains("note C") + ); + + let location = Location::for_file(&app.data_dir, &app.project, &root.join("docs/a.md")); + let record: serde_json::Value = + serde_json::from_slice(&std::fs::read(location.record).expect("record")).expect("JSON"); + let ids: Vec = record["deliveries"] + .as_array() + .expect("deliveries") + .iter() + .map(|d| d["annotation_ids"].as_array().expect("ids").len()) + .collect(); + assert_eq!(ids, [2, 1, 1, 3]); + let history = + std::fs::read_to_string(app.data_dir.join("feedback").join(&app.project).join("index.jsonl")) + .expect("history"); + let records: Vec = + history.lines().map(|s| serde_json::from_str(s).expect("history JSON")).collect(); + assert_eq!(records.len(), 4); + assert_eq!(records[1]["annotations"].as_array().expect("selected annotations").len(), 1); + assert_eq!(records[1]["annotations"][0]["text"], "note C"); + assert_eq!(records[2]["annotations"][0]["text"], "note B revised"); + std::fs::remove_dir_all(root).expect("cleanup"); +} + +#[test] +fn the_zero_pending_button_stays_visible_and_does_not_send_or_record_another_delivery() { + let (root, mut app, delivery) = file_app("nothing-new"); + press(&mut app, 'E'); + assert!(delivery.calls.borrow().is_empty()); + assert_eq!(app.status.as_deref(), Some("nothing new to send")); + app.add_quote_annotation("one", Kind::Comment, "A".into()).expect("A"); + press(&mut app, 'E'); + let location = Location::for_file(&app.data_dir, &app.project, &root.join("docs/a.md")); + let before = std::fs::read(&location.record).expect("record"); + let screen = draw(&mut app, 80, 24); + assert!(screen.contains("Send 0 new")); + let rect = app.geometry.send_button.expect("still clickable"); + click(&mut app, rect); + assert_eq!(delivery.calls.borrow().len(), 1); + assert_eq!(std::fs::read(&location.record).expect("same record"), before); + assert!(draw(&mut app, 80, 24).contains("nothing new to send")); + press(&mut app, 'q'); + assert!(app.quit, "no pending feedback means no quit confirmation"); + assert_eq!(app.mode, Mode::Browse); + std::fs::remove_dir_all(root).expect("cleanup"); +} + +#[test] +fn unsuccessful_sends_leave_the_record_pending_and_offer_retry() { + for outcome in [Outcome::Blocked, Outcome::Unavailable, Outcome::Failed] { + let (root, mut app, delivery) = file_app("failed-send"); + app.add_quote_annotation("one", Kind::Comment, "keep pending".into()).expect("annotation"); + let location = Location::for_file(&app.data_dir, &app.project, &root.join("docs/a.md")); + let before = std::fs::read(&location.record).expect("record"); + delivery.outcome.set(outcome); + press(&mut app, 'E'); + assert_eq!(std::fs::read(&location.record).expect("record"), before); + assert_eq!(app.send_count(), 1); + let status = app.status.as_deref().expect("failure status"); + assert!(status.contains("E retry"), "{status}"); + assert!(!status.contains("copied"), "clipboard was disabled"); + assert!(!app.data_dir.join("feedback").exists(), "no submission history for a failure"); + reopen(&mut app); + assert_eq!(app.send_count(), 1); + delivery.outcome.set(Outcome::Success); + press(&mut app, 'E'); + assert_eq!(app.send_count(), 0); + std::fs::remove_dir_all(root).expect("cleanup"); + } +} + +#[test] +fn folder_counts_and_delivery_cover_collapsed_files_but_exclude_orphans_and_siblings() { + let (root, mut app, delivery) = folder_app("folder-scope"); + app.add_quote_annotation("one", Kind::Comment, "already sent A".into()).expect("A"); + press(&mut app, 'E'); + app.add_quote_annotation("three", Kind::Comment, "new C".into()).expect("C"); + std::fs::create_dir_all(root.join("docs/deep")).expect("subdir"); + let nested = root.join("docs/deep/b.md"); + std::fs::write(&nested, "beta\n\ngone\n").expect("nested file"); + let (doc, mut store) = app.load_review_file(&nested).expect("nested review"); + store.add(&doc, 0..4, "beta".into(), Kind::Comment, "nested D".into()).expect("D"); + store.add(&doc, 6..10, "gone".into(), Kind::Comment, "orphaned note".into()).expect("orphan"); + std::fs::write(&nested, "beta\n").expect("remove orphan's quote"); + let outside = root.join("sibling.md"); + std::fs::write(&outside, "outside\n").expect("sibling"); + let (doc, mut store) = app.load_review_file(&outside).expect("sibling store"); + store.add(&doc, 0..7, "outside".into(), Kind::Comment, "other folder".into()).expect("sibling note"); + let archived = root.join("docs/deep/c.md"); + std::fs::write(&archived, "finished\n").expect("archive-only file"); + let (doc, mut store) = app.load_review_file(&archived).expect("archived store"); + store.add(&doc, 0..8, "finished".into(), Kind::Comment, "finished note".into()).expect("note"); + let id = store.placed()[0].annotation.id.clone(); + store.record_delivery("test agent", &[id]).expect("sent"); + store.archive_sent().expect("archive"); + + app.refresh_review_counts().expect("refresh counts"); + assert_eq!(app.send_count(), 2); + assert_eq!(app.pending_file_count(), 2); + assert_eq!(app.review_counts().archived, 1); + assert!(app.send_label().contains("2 new across 2 files")); + assert!( + !app.tree.as_ref().expect("tree").rows.iter().any(|r| r.path == nested), + "nested file is not listed" + ); + press(&mut app, 'E'); + let calls = delivery.calls.borrow(); + let sent = calls.last().expect("folder feedback"); + assert!(sent.contains("new C") && sent.contains("nested D"), "{sent}"); + for excluded in ["already sent A", "orphaned note", "other folder", "finished note", "No annotations."] { + assert!(!sent.contains(excluded), "{excluded} was included: {sent}"); + } + assert!(app.status.as_deref().expect("status").contains("2 annotation(s) across 2 files")); + let (_, nested_store) = app.load_review_file(&nested).expect("reload nested"); + assert!(!nested_store.all_delivered(), "the unsent orphan was not recorded as sent"); + assert_eq!(nested_store.placed().len(), 1); + assert!(!nested_store.is_pending(nested_store.placed()[0].annotation)); + let (_, outside_store) = app.load_review_file(&outside).expect("reload sibling"); + assert!(!outside_store.all_delivered()); + std::fs::remove_dir_all(root).expect("cleanup"); +} + +#[test] +fn reply_reviews_keep_sending_the_whole_transient_review() { + let (root, mut app, delivery) = file_app("reply-scope"); + let source = DocumentSource::new( + "one\n\ntwo\n".into(), + "agent reply", + true, + Provenance::AgentMessage { + host: "claude".into(), + session: None, + message_id: Some("message-1".into()), + }, + ); + app.open = Open::new(source, 80, &app.data_dir, &app.project).expect("reply"); + app.add_quote_annotation("one", Kind::Comment, "A".into()).expect("A"); + press(&mut app, 'E'); + app.add_quote_annotation("two", Kind::Comment, "B".into()).expect("B"); + press(&mut app, 'E'); + let calls = delivery.calls.borrow(); + assert!(calls[1].contains("> A") && calls[1].contains("> B")); + press(&mut app, 'F'); + assert_eq!(app.open.store.len(), 2); + assert!(app.open.store.archived().is_empty()); + assert!(app.open.store.is_transient()); + assert!(!draw(&mut app, 80, 24).contains("Finish review")); + std::fs::remove_dir_all(root).expect("cleanup"); +} diff --git a/crates/plannotator-tui/src/app/header.rs b/crates/plannotator-tui/src/app/header.rs index 2d2bf42..8206bad 100644 --- a/crates/plannotator-tui/src/app/header.rs +++ b/crates/plannotator-tui/src/app/header.rs @@ -1,6 +1,5 @@ -//! The header row: the Send button, right-aligned, and nothing else. The pane label -//! (Herdr's) names the app; the footer names the file. The button is clickable, so its rect -//! is recorded for hit-testing. +//! Visible review actions. Buttons wrap onto another row in a narrow pane instead of +//! disappearing or hiding a single action behind a menu. use ratatui::Frame; use ratatui::layout::Rect; @@ -16,20 +15,80 @@ const IDLE_BG: Color = Color::Indexed(238); const SENT_BG: Color = Color::Indexed(22); const BLOCKED_BG: Color = Color::Indexed(58); +#[derive(Debug, Clone, Copy)] +enum Button { + Send, + Resend, + Finish, + Archive, +} + impl App { + fn header_buttons(&self, width: u16) -> Vec<(Button, String, Rect)> { + if width == 0 { + return Vec::new(); + } + let mut labels = vec![(Button::Send, self.send_label())]; + if self.is_file_review() { + let counts = self.review_counts(); + labels.extend([ + (Button::Resend, format!("Resend all ({} sent) (R)", counts.sent)), + (Button::Finish, "Finish review (F)".into()), + (Button::Archive, format!("Archive {} (H)", counts.archived)), + ]); + } + let mut right = width; + let mut y = 0; + labels + .into_iter() + .map(|(button, label)| { + let label = format!(" {label} "); + let button_width = label.width().min(usize::from(width)) as u16; + if button_width > right { + y += 1; + right = width; + } + let rect = Rect { x: right - button_width, y, width: button_width, height: 1 }; + right = rect.x.saturating_sub(1); + (button, label, rect) + }) + .collect() + } + + pub(super) fn header_height(&self, width: u16) -> u16 { + self.header_buttons(width).last().map_or(1, |(_, _, rect)| rect.y + 1) + } + pub(super) fn draw_header(&mut self, frame: &mut Frame, area: Rect) { - let button = format!(" {} ", self.send_label()); - let width = button.width() as u16; - self.geometry.send_button = - (area.width >= width).then(|| Rect { x: area.right() - width, y: area.y, width, height: 1 }); - if let Some(rect) = self.geometry.send_button { - let span = Span::styled(button, self.button_style()); - frame.buffer_mut().set_span(rect.x, rect.y, &span, rect.width); + for (button, label, mut rect) in self.header_buttons(area.width) { + if rect.y >= area.height { + continue; + } + rect.x += area.x; + rect.y += area.y; + let style = match button { + Button::Send => { + self.geometry.send_button = Some(rect); + self.button_style() + } + Button::Resend => { + self.geometry.resend_button = Some(rect); + Style::new().fg(Color::Cyan).bg(IDLE_BG) + } + Button::Finish => { + self.geometry.finish_button = Some(rect); + Style::new().fg(Color::Cyan).bg(IDLE_BG) + } + Button::Archive => { + self.geometry.archive_button = Some(rect); + Style::new().fg(Color::Cyan).bg(IDLE_BG) + } + }; + frame.buffer_mut().set_span(rect.x, rect.y, &Span::styled(label, style), rect.width); } } - /// Teal when there is something to send, grey at zero, green once sent, yellow when the - /// agent refused it. + /// At zero the button is still drawn and clickable; it reports the no-op in the footer. fn button_style(&self) -> Style { match &self.send_state { SendState::Ready if self.send_count() == 0 => { diff --git a/crates/plannotator-tui/src/app/input.rs b/crates/plannotator-tui/src/app/input.rs index 6761312..f09b677 100644 --- a/crates/plannotator-tui/src/app/input.rs +++ b/crates/plannotator-tui/src/app/input.rs @@ -19,6 +19,10 @@ impl App { Mode::Browse => self.browse_key(*key), Mode::ConfirmQuit => self.confirm_quit_key(*key), Mode::Pick => self.pick_key(*key), + Mode::Archive => { + self.archive_key(*key); + Ok(()) + } Mode::Compose | Mode::Edit(_) => self.text_key(*key), }, // A paste lands in the comment box verbatim, newlines included; anywhere else @@ -29,6 +33,10 @@ impl App { } Event::Mouse(mouse) if self.mode == Mode::Browse => self.mouse(*mouse), Event::Mouse(mouse) if self.mode == Mode::Pick => self.pick_mouse(*mouse), + Event::Mouse(mouse) if self.mode == Mode::Archive => { + self.archive_mouse(*mouse); + Ok(()) + } _ => Ok(()), } } @@ -45,6 +53,19 @@ impl App { return Ok(()); } (KeyCode::Char('E'), _) => return self.send_feedback(), + (KeyCode::Char('R'), _) if self.is_file_review() => return self.resend_all(), + (KeyCode::Char('F'), _) if self.is_file_review() => { + self.finish_review(); + return Ok(()); + } + (KeyCode::Char('H'), _) if self.is_file_review() => { + self.open_archive(); + return Ok(()); + } + (KeyCode::Char('U'), _) if self.is_file_review() => { + self.undo_finish_review(); + return Ok(()); + } (KeyCode::Char('t'), _) => { self.toggle_tree(self.geometry.doc.width + self.geometry.tree.width + GUTTER); return Ok(()); @@ -274,7 +295,7 @@ impl App { self.status = Some("annotation updated".into()); } } - Mode::Compose | Mode::Browse | Mode::ConfirmQuit | Mode::Pick => { + Mode::Compose | Mode::Browse | Mode::ConfirmQuit | Mode::Pick | Mode::Archive => { if !body.is_empty() && let Some(pending) = self.pending.take() { @@ -300,6 +321,24 @@ impl App { if self.send_button_hit(mouse.column, mouse.row) { return self.send_feedback(); } + let hit = |rect: Option| { + rect.is_some_and(|r| mouse.row == r.y && mouse.column >= r.x && mouse.column < r.right()) + }; + if hit(self.geometry.resend_button) { + return self.resend_all(); + } + if hit(self.geometry.finish_button) { + self.finish_review(); + return Ok(()); + } + if hit(self.geometry.archive_button) { + self.open_archive(); + return Ok(()); + } + if hit(self.geometry.undo_button) { + self.undo_finish_review(); + return Ok(()); + } if let Some(kind) = self.toolbar_hit(mouse.column, mouse.row) { return self.act(kind); } diff --git a/crates/plannotator-tui/src/app/mod.rs b/crates/plannotator-tui/src/app/mod.rs index 2dc6927..4d9e080 100644 --- a/crates/plannotator-tui/src/app/mod.rs +++ b/crates/plannotator-tui/src/app/mod.rs @@ -1,19 +1,23 @@ //! Application state. Input handling lives in `input`, drawing in `draw`; this module owns //! the data they share and the operations that change it. +mod archive_view; mod compose; mod draw; +mod feedback; mod header; mod input; mod pick; +mod review; +#[cfg(test)] +mod review_test_support; mod selection; mod send; #[cfg(test)] mod tests; use std::collections::HashMap; -use std::fmt::Write as _; use std::ops::Range; use std::path::{Path, PathBuf}; @@ -23,7 +27,6 @@ use ratatui::layout::Rect; use crate::delivery::Delivery; use crate::doc::Document; -use crate::export; use crate::layout::DocLayout; use crate::store::{Location, Store}; use crate::tree::Tree; @@ -52,6 +55,8 @@ enum Mode { ConfirmQuit, /// Choosing which of the agent's recent messages to review. Pick, + /// Restoring annotations from finished file reviews. + Archive, } /// Which pane keyboard input goes to. @@ -73,6 +78,11 @@ struct Geometry { bubbles: Vec<(Rect, String)>, /// The header's Send button; `None` when the header was too narrow for it. send_button: Option, + resend_button: Option, + finish_button: Option, + archive_button: Option, + undo_button: Option, + archive_rows: Vec<(Rect, usize)>, /// Picker rows drawn last frame, with their candidate index. pick_rows: Vec<(Rect, usize)>, } @@ -124,6 +134,10 @@ pub(crate) struct App { tree_visible: Option, delivery: Box, send_state: SendState, + folder_counts: HashMap, + undo_archive: Vec, + archive_items: Vec, + archive_cursor: usize, focus: Focus, scroll: usize, selected: usize, @@ -195,6 +209,10 @@ impl App { tree_visible: None, delivery, send_state, + folder_counts: HashMap::new(), + undo_archive: Vec::new(), + archive_items: Vec::new(), + archive_cursor: 0, focus: Focus::Document, scroll: 0, selected: 0, @@ -247,10 +265,11 @@ impl App { if let Some(path) = &first { app.open = Open::new(read_file(path)?, width, &app.data_dir, &app.project)?; } - app.derive_send_state(); app.refresh_counts(&mut tree); app.tree_cursor = first.as_deref().and_then(|p| tree.position(p)).unwrap_or(0); app.tree = Some(tree); + app.refresh_review_counts()?; + app.derive_send_state(); Ok(app) } @@ -290,6 +309,8 @@ impl App { self.refresh_counts(&mut tree); self.tree = Some(tree); result?; + self.refresh_review_counts()?; + self.derive_send_state(); } return Ok(()); } @@ -300,6 +321,7 @@ impl App { } let width = self.open.layout.width; self.open = Open::new(read_file(&path)?, width, &self.data_dir, &self.project)?; + self.update_open_review_counts(); self.derive_send_state(); self.scroll = 0; self.selected = 0; @@ -366,95 +388,6 @@ impl App { Ok(()) } - /// The feedback document for every placed annotation of the open file. - pub(crate) fn feedback(&self) -> String { - Self::feedback_for(&self.open, &self.open.source.name) - } - - fn feedback_for(open: &Open, name: &str) -> String { - let source = &open.doc.source; - let entries: Vec> = open - .store - .placed() - .into_iter() - .map(|p| export::Entry { - annotation: p.annotation, - lines: export::line_span(source, p.range), - range: p.range.clone(), - }) - .collect(); - export::feedback(source, name, &entries) - } - - /// Feedback for every annotated file in the folder, one `# Annotations on ` block each. - pub(crate) fn folder_feedback(&self) -> Result { - let Some(tree) = &self.tree else { return Ok(self.feedback()) }; - let width = self.open.layout.width; - let mut out = String::new(); - for path in self.annotated_files() { - let open = Open::new(read_file(&path)?, width, &self.data_dir, &self.project)?; - let relative = path.strip_prefix(tree.root()).unwrap_or(&path); - let _ = writeln!(out, "{}", Self::feedback_for(&open, &relative.display().to_string())); - } - Ok(if out.is_empty() { "No annotations.".to_owned() } else { out }) - } - - /// Paths of every annotated file in the folder: the project's records (which carry their - /// document path since 0.5.0) plus any listed tree row with a count, so nothing depends - /// on which directories happen to be expanded. - fn annotated_files(&self) -> Vec { - let Some(tree) = &self.tree else { return Vec::new() }; - let mut found = Store::annotated_documents(&self.data_dir, &self.project); - for row in tree.rows.iter().filter(|r| !r.is_dir && r.annotations > 0) { - found.push(row.path.clone()); - } - found.sort(); - found.dedup(); - found.retain(|p| p.is_file()); - found - } - - fn is_open(&self, path: &Path) -> bool { - matches!(&self.open.source.provenance, Provenance::File { path: p } if p == path) - } - - /// Remember the send on every file it covered: the open one in memory, the rest on disk. - fn record_delivery(&mut self, target: &str) -> Result<()> { - if self.tree.is_none() { - return self.open.store.record_delivery(target); - } - let width = self.open.layout.width; - for path in self.annotated_files() { - if self.is_open(&path) { - self.open.store.record_delivery(target)?; - } else { - let mut open = Open::new(read_file(&path)?, width, &self.data_dir, &self.project)?; - open.store.record_delivery(target)?; - } - } - Ok(()) - } - - /// True when every annotated file in the folder has been sent since it last changed. - fn folder_all_delivered(&self) -> Result { - let files = self.annotated_files(); - if files.is_empty() { - return Ok(false); - } - let width = self.open.layout.width; - for path in files { - let delivered = if self.is_open(&path) { - self.open.store.all_delivered() - } else { - Open::new(read_file(&path)?, width, &self.data_dir, &self.project)?.store.all_delivered() - }; - if !delivered { - return Ok(false); - } - } - Ok(true) - } - fn clear_selection(&mut self) { self.selection = None; self.pending = None; @@ -533,7 +466,11 @@ impl App { self.open.source = read_file(&path)?; self.open.doc = Document::parse(self.open.source.content.clone()); self.open.layout = DocLayout::build(&self.open.doc, self.open.layout.width); - self.open.store.resolve_all(&self.open.doc); + self.open.store = + Store::load(&Location::for_file(&self.data_dir, &self.project, &path), &self.open.doc)?; + self.refresh_review_counts()?; + self.derive_send_state(); + self.sync_tree_counts(); self.clear_selection(); self.selected = self.selected.min(self.open.doc.blocks.len().saturating_sub(1)); self.status = Some(format!("reloaded ยท {} orphaned", self.open.store.orphans())); diff --git a/crates/plannotator-tui/src/app/review.rs b/crates/plannotator-tui/src/app/review.rs new file mode 100644 index 0000000..364b084 --- /dev/null +++ b/crates/plannotator-tui/src/app/review.rs @@ -0,0 +1,186 @@ +//! Finish a file/folder review, undo it, or restore individual archived annotations. + +use std::fmt::Write as _; +use std::path::{Path, PathBuf}; + +use anyhow::Result; +use plannotator_tui_schema::Annotation; + +use super::feedback::ReviewCounts; +use super::{App, Focus, Mode}; + +#[cfg(test)] +mod tests; + +#[derive(Debug)] +pub(super) struct ArchivedBatch { + path: PathBuf, + ids: Vec, +} + +#[derive(Debug, Clone)] +pub(super) struct ArchivedItem { + pub(super) path: PathBuf, + pub(super) annotation: Annotation, +} + +impl App { + pub(super) fn finish_review(&mut self) { + if !self.is_file_review() { + return; + } + let mut batches = Vec::new(); + let mut errors = Vec::new(); + let mut count = 0; + for path in self.review_files() { + match self.archive_review_file(&path) { + Ok(ids) if !ids.is_empty() => { + count += ids.len(); + batches.push(ArchivedBatch { path, ids }); + } + Ok(_) => {} + Err(err) => errors.push(format!("{}: {err:#}", self.review_file_name(&path))), + } + } + if !batches.is_empty() { + self.undo_archive = batches; + } + self.after_review_change(); + let status = if count == 0 && errors.is_empty() { + "no sent annotations to archive".into() + } else { + format!("archived {count} annotation(s)") + }; + self.status = Some(with_errors(status, "archive", &errors)); + } + + fn archive_review_file(&mut self, path: &Path) -> Result> { + if self.is_open(path) { + return self.open.store.archive_sent(); + } + let (_, mut store) = self.load_review_file(path)?; + let ids = store.archive_sent()?; + self.folder_counts.insert(path.to_path_buf(), ReviewCounts::for_store(&store)); + Ok(ids) + } + + pub(super) fn undo_finish_review(&mut self) { + if self.undo_archive.is_empty() { + self.status = Some("nothing to undo".into()); + return; + } + let mut count = 0; + let mut errors = Vec::new(); + for batch in std::mem::take(&mut self.undo_archive) { + match self.restore_review_file(&batch.path, &batch.ids) { + Ok(restored) => count += restored, + Err(err) => { + errors.push(format!("{}: {err:#}", self.review_file_name(&batch.path))); + self.undo_archive.push(batch); + } + } + } + self.after_review_change(); + self.status = Some(with_errors(format!("restored {count} annotation(s)"), "restore", &errors)); + } + + fn restore_review_file(&mut self, path: &Path, ids: &[String]) -> Result { + if self.is_open(path) { + return self.open.store.restore_archived(&self.open.doc, ids); + } + let (doc, mut store) = self.load_review_file(path)?; + let restored = store.restore_archived(&doc, ids)?; + self.folder_counts.insert(path.to_path_buf(), ReviewCounts::for_store(&store)); + Ok(restored) + } + + fn after_review_change(&mut self) { + let remaining = self.open.store.placed().len(); + self.rail_cursor = self.rail_cursor.min(remaining.saturating_sub(1)); + if remaining == 0 && self.focus == Focus::Rail { + self.focus = Focus::Document; + } + self.clear_selection(); + self.mark_unsent(); + self.sync_tree_counts(); + } + + pub(super) fn open_archive(&mut self) { + if !self.is_file_review() { + return; + } + let errors = self.refresh_archive_items(); + if !errors.is_empty() { + self.status = Some(with_errors(String::new(), "read archive", &errors)); + } + self.archive_cursor = 0; + self.mode = Mode::Archive; + } + + fn refresh_archive_items(&mut self) -> Vec { + let mut items = Vec::new(); + let mut errors = Vec::new(); + for path in self.review_files() { + let annotations = if self.is_open(&path) { + self.open.store.archived().to_vec() + } else { + match self.load_review_file(&path) { + Ok((_, store)) => store.archived().to_vec(), + Err(err) => { + errors.push(format!("{}: {err:#}", self.review_file_name(&path))); + continue; + } + } + }; + items.extend( + annotations + .into_iter() + .rev() + .map(|annotation| ArchivedItem { path: path.clone(), annotation }), + ); + } + self.archive_items = items; + self.archive_cursor = self.archive_cursor.min(self.archive_items.len().saturating_sub(1)); + errors + } + + pub(super) fn restore_selected_archived(&mut self) { + let Some(item) = self.archive_items.get(self.archive_cursor).cloned() else { return }; + match self.restore_review_file(&item.path, std::slice::from_ref(&item.annotation.id)) { + Ok(0) => { + self.status = Some("could not restore: an annotation with this id is already active".into()); + } + Ok(_) => { + for batch in &mut self.undo_archive { + if batch.path == item.path { + batch.ids.retain(|id| id != &item.annotation.id); + } + } + self.undo_archive.retain(|batch| !batch.ids.is_empty()); + self.status = Some(format!("restored annotation in {}", self.review_file_name(&item.path))); + } + Err(err) => { + self.status = + Some(format!("could not restore {}: {err:#}", self.review_file_name(&item.path))); + } + } + self.after_review_change(); + let errors = self.refresh_archive_items(); + if !errors.is_empty() { + self.status = Some(with_errors(self.status.take().unwrap_or_default(), "read archive", &errors)); + } + if self.archive_items.is_empty() { + self.mode = Mode::Browse; + } + } +} + +fn with_errors(mut status: String, action: &str, errors: &[String]) -> String { + if !errors.is_empty() { + if !status.is_empty() { + status.push_str("; "); + } + let _ = write!(status, "could not {action}: {}", errors.join("; ")); + } + status +} diff --git a/crates/plannotator-tui/src/app/review/tests.rs b/crates/plannotator-tui/src/app/review/tests.rs new file mode 100644 index 0000000..6770adb --- /dev/null +++ b/crates/plannotator-tui/src/app/review/tests.rs @@ -0,0 +1,240 @@ +#![allow(clippy::expect_used, clippy::indexing_slicing, reason = "tests assert by panicking")] + +use plannotator_tui_schema::Kind; +use ratatui::crossterm::event::{Event, KeyCode, KeyEvent}; + +use crate::app::review_test_support::{click, draw, file_app, folder_app, press, reopen}; +use crate::app::{Focus, Mode}; +use crate::store::Location; + +#[test] +fn finish_undo_and_restore_keep_pending_edits_and_preserve_sent_status() { + let (root, mut app, delivery) = file_app("finish"); + app.add_quote_annotation("one", Kind::Comment, "A".into()).expect("A"); + app.add_quote_annotation("two", Kind::Comment, "B".into()).expect("B"); + let b = app.open.store.placed()[1].annotation.id.clone(); + press(&mut app, 'E'); + app.open.store.edit_body(&b, "B revised".into()).expect("edit B"); + app.add_quote_annotation("three", Kind::Comment, "C".into()).expect("C"); + press(&mut app, 'F'); + assert_eq!(app.open.store.len(), 2); + assert_eq!(app.open.store.archived().len(), 1); + assert_eq!(app.send_count(), 2); + assert_eq!(delivery.calls.borrow().len(), 1); + assert!(!app.quit); + let screen = draw(&mut app, 80, 24); + assert!(screen.contains("archived 1 annotation(s)") && screen.contains("U Undo"), "{screen}"); + let undo = app.geometry.undo_button.expect("undo button"); + click(&mut app, undo); + assert_eq!(app.open.store.len(), 3); + assert_eq!(app.send_count(), 2, "undo did not make A pending"); + assert!(app.open.store.archived().is_empty()); + + press(&mut app, 'F'); + reopen(&mut app); + app.undo_archive.clear(); // a new process has no in-memory undo, but the archive persists + press(&mut app, 'H'); + assert_eq!(app.mode, Mode::Archive); + assert_eq!(app.archive_items.len(), 1); + assert!(draw(&mut app, 80, 24).contains("Archived annotations (1)")); + app.handle_event(&Event::Key(KeyEvent::from(KeyCode::Enter))).expect("restore"); + assert_eq!(app.mode, Mode::Browse); + assert_eq!(app.send_count(), 2); + press(&mut app, 'E'); + let calls = delivery.calls.borrow(); + assert_eq!(calls.len(), 2); + assert!(!calls[1].contains("> A")); + assert!(calls[1].contains("B revised") && calls[1].contains("> C")); + std::fs::remove_dir_all(root).expect("cleanup"); +} + +#[test] +fn sent_markers_clear_on_edit_and_review_shortcuts_are_text_while_composing() { + let (root, mut app, _) = file_app("edit-ui"); + press(&mut app, 'c'); + assert_eq!(app.mode, Mode::Compose); + for ch in "FRHU".chars() { + press(&mut app, ch); + } + app.handle_event(&Event::Key(KeyEvent::from(KeyCode::Enter))).expect("save"); + assert_eq!(app.open.store.placed()[0].annotation.body, "FRHU"); + press(&mut app, 'E'); + assert!(draw(&mut app, 100, 24).contains(" ยท sent")); + app.focus = Focus::Rail; + press(&mut app, 'e'); + press(&mut app, '!'); + app.handle_event(&Event::Key(KeyEvent::from(KeyCode::Enter))).expect("save edit"); + assert_eq!(app.send_count(), 1); + assert!(!draw(&mut app, 100, 24).contains(" ยท sent")); + press(&mut app, 'q'); + assert_eq!(app.mode, Mode::ConfirmQuit, "an edited sent note still needs sending"); + std::fs::remove_dir_all(root).expect("cleanup"); +} + +#[test] +fn every_review_action_stays_visible_and_clickable_in_a_narrow_pane() { + let (root, mut app, _) = file_app("buttons"); + app.add_quote_annotation("one", Kind::Comment, "note".into()).expect("note"); + press(&mut app, 'E'); + for width in [80, 40] { + let screen = draw(&mut app, width, 24); + let buttons = [ + app.geometry.send_button, + app.geometry.resend_button, + app.geometry.finish_button, + app.geometry.archive_button, + ]; + for rect in buttons.into_iter().map(|r| r.expect("visible button")) { + assert!(rect.right() <= width && rect.bottom() < 24, "{screen}"); + } + assert!( + screen.contains("Finish review") && screen.contains("Archive 0") && screen.contains("Resend all"), + "{screen}" + ); + } + let finish = app.geometry.finish_button.expect("finish"); + click(&mut app, finish); + assert_eq!(app.open.store.len(), 0); + draw(&mut app, 40, 24); + let archive = app.geometry.archive_button.expect("archive"); + click(&mut app, archive); + assert_eq!(app.mode, Mode::Archive); + draw(&mut app, 40, 24); + let row = app.geometry.archive_rows[0].0; + click(&mut app, row); + assert_eq!(app.open.store.len(), 1); + assert_eq!(app.send_count(), 0); + std::fs::remove_dir_all(root).expect("cleanup"); +} + +#[test] +fn the_archive_picker_scrolls_and_restores_the_clicked_annotation() { + let (root, mut app, _) = file_app("archive-scroll"); + for index in 0..10 { + app.add_quote_annotation("one", Kind::Comment, format!("note {index}")).expect("note"); + } + press(&mut app, 'E'); + press(&mut app, 'F'); + press(&mut app, 'H'); + for _ in 0..7 { + press(&mut app, 'j'); + } + assert_eq!(app.archive_cursor, 7); + let selected = app.archive_items[7].annotation.clone(); + let screen = draw(&mut app, 80, 14); + assert!(screen.contains(&selected.body), "{screen}"); + let rect = app.geometry.archive_rows.iter().find(|(_, i)| *i == 7).expect("selected row visible").0; + click(&mut app, rect); + assert_eq!(app.open.store.placed()[0].annotation, &selected); + assert_eq!(app.open.store.archived().len(), 9); + assert_eq!(app.send_count(), 0); + assert_eq!(app.mode, Mode::Archive); + app.handle_event(&Event::Key(KeyEvent::from(KeyCode::Esc))).expect("close"); + assert_eq!(app.mode, Mode::Browse); + std::fs::remove_dir_all(root).expect("cleanup"); +} + +#[test] +fn folder_finish_reports_partial_failure_and_undo_covers_only_committed_archives() { + let (root, mut app, _) = folder_app("partial-archive"); + app.add_quote_annotation("one", Kind::Comment, "A".into()).expect("A"); + let other = root.join("docs/b.md"); + std::fs::write(&other, "beta\n\nnew\n").expect("B file"); + let (doc, mut store) = app.load_review_file(&other).expect("B store"); + store.add(&doc, 0..4, "beta".into(), Kind::Comment, "B".into()).expect("B"); + app.refresh_review_counts().expect("refresh"); + press(&mut app, 'E'); + let (doc, mut store) = app.load_review_file(&other).expect("sent B store"); + store.add(&doc, 6..9, "new".into(), Kind::Comment, "C pending".into()).expect("C"); + app.refresh_review_counts().expect("refresh"); + let location = Location::for_file(&app.data_dir, &app.project, &other); + let blocked_tmp = location.record.with_extension("json.tmp"); + std::fs::create_dir(&blocked_tmp).expect("block B archive"); + press(&mut app, 'F'); + let status = app.status.as_deref().expect("status"); + assert!( + status.starts_with("archived 1 annotation(s)") + && status.contains("could not archive") + && status.contains("b.md"), + "{status}" + ); + assert_eq!(app.open.store.archived().len(), 1); + assert_eq!(app.load_review_file(&other).expect("B intact").1.len(), 2); + assert_eq!(app.send_count(), 1); + press(&mut app, 'U'); + assert_eq!(app.open.store.len(), 1); + assert_eq!(app.send_count(), 1); + std::fs::remove_dir(&blocked_tmp).expect("unblock"); + press(&mut app, 'F'); + assert_eq!(app.review_counts().archived, 2); + assert_eq!(app.send_count(), 1); + press(&mut app, 'U'); + assert_eq!(app.review_counts().archived, 0); + assert_eq!(app.review_counts().sent, 2); + assert_eq!(app.send_count(), 1); + std::fs::remove_dir_all(root).expect("cleanup"); +} + +#[test] +fn a_failed_undo_stays_available_for_retry_without_losing_the_archive() { + let (root, mut app, _) = file_app("undo-failure"); + app.add_quote_annotation("one", Kind::Comment, "A".into()).expect("A"); + press(&mut app, 'E'); + press(&mut app, 'F'); + let location = Location::for_file(&app.data_dir, &app.project, &root.join("docs/a.md")); + let blocked_tmp = location.record.with_extension("json.tmp"); + std::fs::create_dir(&blocked_tmp).expect("block restore"); + press(&mut app, 'U'); + assert!(!app.undo_archive.is_empty()); + assert_eq!(app.open.store.len(), 0); + assert_eq!(app.open.store.archived().len(), 1); + assert!(app.status.as_deref().expect("status").contains("could not restore")); + std::fs::remove_dir(&blocked_tmp).expect("unblock"); + press(&mut app, 'U'); + assert!(app.undo_archive.is_empty()); + assert_eq!(app.open.store.len(), 1); + assert_eq!(app.send_count(), 0); + std::fs::remove_dir_all(root).expect("cleanup"); +} + +#[test] +fn a_deleted_files_archive_is_still_visible_and_restores_when_its_source_returns() { + let (root, mut app, _) = folder_app("deleted-source"); + let path = root.join("docs/b.md"); + std::fs::write(&path, "beta\n").expect("source"); + let (doc, mut store) = app.load_review_file(&path).expect("store"); + store.add(&doc, 0..4, "beta".into(), Kind::Comment, "keep this feedback".into()).expect("note"); + press(&mut app, 'E'); + press(&mut app, 'F'); + std::fs::remove_file(&path).expect("source removed"); + app.refresh_review_counts().expect("refresh"); + assert_eq!(app.review_counts().archived, 1); + press(&mut app, 'H'); + assert_eq!(app.archive_items.len(), 1); + app.handle_event(&Event::Key(KeyEvent::from(KeyCode::Enter))).expect("restore"); + assert_eq!(app.send_count(), 0); + assert_eq!(app.load_review_file(&path).expect("missing source").1.orphans(), 1); + std::fs::write(&path, "beta\n").expect("source returns"); + let (_, store) = app.load_review_file(&path).expect("reopen source"); + assert_eq!(store.placed().len(), 1); + assert!(store.all_delivered()); + std::fs::remove_dir_all(root).expect("cleanup"); +} + +#[test] +fn finished_notes_are_recoverable_with_submission_history_disabled() { + let (root, mut app, _) = file_app("history-disabled"); + std::fs::create_dir_all(&app.data_dir).expect("data dir"); + std::fs::write(app.data_dir.join("config.json"), r#"{"feedbackHistory":false}"#) + .expect("disable history"); + app.add_quote_annotation("one", Kind::Comment, "recover me".into()).expect("note"); + press(&mut app, 'E'); + assert!(!app.data_dir.join("feedback").exists()); + press(&mut app, 'F'); + reopen(&mut app); + press(&mut app, 'H'); + app.handle_event(&Event::Key(KeyEvent::from(KeyCode::Enter))).expect("restore"); + assert_eq!(app.open.store.placed()[0].annotation.body, "recover me"); + assert_eq!(app.send_count(), 0); + std::fs::remove_dir_all(root).expect("cleanup"); +} diff --git a/crates/plannotator-tui/src/app/review_test_support.rs b/crates/plannotator-tui/src/app/review_test_support.rs new file mode 100644 index 0000000..7de2a6b --- /dev/null +++ b/crates/plannotator-tui/src/app/review_test_support.rs @@ -0,0 +1,117 @@ +//! Isolated file reviews and a recording stand-in for the external delivery transport. + +#![allow(clippy::expect_used, reason = "test fixtures assert by panicking")] + +use std::cell::{Cell, RefCell}; +use std::path::PathBuf; +use std::rc::Rc; +use std::sync::atomic::{AtomicUsize, Ordering}; + +use plannotator_tui_schema::{DocumentSource, Provenance}; +use ratatui::Terminal; +use ratatui::backend::TestBackend; +use ratatui::crossterm::event::{ + Event, KeyCode, KeyEvent, KeyModifiers, MouseButton, MouseEvent, MouseEventKind, +}; +use ratatui::layout::Rect; + +use super::{App, Open, read_file}; +use crate::delivery::{Delivery, DeliveryError}; +use crate::tree::Tree; + +#[derive(Debug, Default, Clone, Copy)] +pub(super) enum Outcome { + #[default] + Success, + Blocked, + Unavailable, + Failed, +} + +#[derive(Debug, Default, Clone)] +pub(super) struct RecordingDelivery { + pub(super) calls: Rc>>, + pub(super) outcome: Rc>, +} + +impl Delivery for RecordingDelivery { + fn describe(&self) -> String { + "test agent".into() + } + fn is_agent(&self) -> bool { + true + } + fn deliver(&self, text: &str) -> Result<(), DeliveryError> { + self.calls.borrow_mut().push(text.to_owned()); + match self.outcome.get() { + Outcome::Success => Ok(()), + Outcome::Blocked => Err(DeliveryError::Blocked("at a dialog".into())), + Outcome::Unavailable => Err(DeliveryError::Unavailable("agent gone".into())), + Outcome::Failed => Err(DeliveryError::Failed(anyhow::anyhow!("transport failed"))), + } + } +} + +pub(super) fn file_app(tag: &str) -> (PathBuf, App, RecordingDelivery) { + static NEXT: AtomicUsize = AtomicUsize::new(0); + let root = std::env::temp_dir().join(format!( + "plannotator-review-app-{tag}-{}-{}", + std::process::id(), + NEXT.fetch_add(1, Ordering::Relaxed) + )); + let _ = std::fs::remove_dir_all(&root); + std::fs::create_dir_all(root.join("docs")).expect("docs"); + let path = root.join("docs/a.md"); + std::fs::write(&path, "# Plan\n\none\n\ntwo\n\nthree\n").expect("document"); + let delivery = RecordingDelivery::default(); + // Do not resolve a persistent source until the app has its private data directory. + let source = DocumentSource::new(String::new(), "fixture", true, Provenance::Stdin); + let mut app = App::open(source, 100, Box::new(delivery.clone())).expect("app"); + app.data_dir = root.join("data"); + app.project = "review-tests".into(); + app.open = Open::new(read_file(&path).expect("file"), 100, &app.data_dir, &app.project).expect("open"); + (root, app, delivery) +} + +pub(super) fn folder_app(tag: &str) -> (PathBuf, App, RecordingDelivery) { + let (root, mut app, delivery) = file_app(tag); + app.tree = Some(Tree::scan(&root.join("docs")).expect("tree")); + app.refresh_review_counts().expect("counts"); + (root, app, delivery) +} + +pub(super) fn press(app: &mut App, ch: char) { + app.handle_event(&Event::Key(KeyEvent::from(KeyCode::Char(ch)))).expect("key"); +} + +pub(super) fn click(app: &mut App, rect: Rect) { + app.handle_event(&Event::Mouse(MouseEvent { + kind: MouseEventKind::Down(MouseButton::Left), + column: rect.x + rect.width / 2, + row: rect.y, + modifiers: KeyModifiers::NONE, + })) + .expect("click"); +} + +pub(super) fn draw(app: &mut App, width: u16, height: u16) -> String { + let mut terminal = Terminal::new(TestBackend::new(width, height)).expect("terminal"); + terminal.draw(|frame| app.draw(frame)).expect("draw"); + let buffer = terminal.backend().buffer(); + (0..height) + .map(|y| { + (0..width) + .filter_map(|x| buffer.cell((x, y))) + .map(ratatui::buffer::Cell::symbol) + .collect::() + }) + .collect::>() + .join("\n") +} + +pub(super) fn reopen(app: &mut App) { + let Provenance::File { path } = &app.open.source.provenance else { return }; + app.open = Open::new(read_file(path).expect("read"), 100, &app.data_dir, &app.project).expect("reopen"); + app.refresh_review_counts().expect("counts"); + app.derive_send_state(); +} diff --git a/crates/plannotator-tui/src/app/send.rs b/crates/plannotator-tui/src/app/send.rs index 116c627..a8f19ac 100644 --- a/crates/plannotator-tui/src/app/send.rs +++ b/crates/plannotator-tui/src/app/send.rs @@ -1,73 +1,123 @@ -//! Sending feedback to the delivery target and the state the Send button shows. +//! Sending feedback and the state of the Send button. + +use std::fmt::Write as _; use anyhow::Result; +use plannotator_tui_schema::Provenance; +use super::feedback::{Feedback, ReviewCounts, SendScope}; use super::{App, Mode}; use crate::delivery::{Clipboard, Delivery as _, DeliveryError}; -use crate::store::Store; -use plannotator_tui_schema::{Kind, Provenance}; -/// What the Send button says. Re-derived from the store on load and file switch. #[derive(Debug, Clone, PartialEq, Eq)] pub(super) enum SendState { - /// Something to send (or nothing yet, in which case the button is dimmed). Ready, - /// Everything on record has been sent; nothing changed since. Sent, - /// The last send was refused because the agent is at a dialog. Blocked(String), } impl App { - /// Send feedback: every annotated file's in folder mode, else the open file's. pub(super) fn send_feedback(&mut self) -> Result<()> { - let text = if self.tree.is_some() { self.folder_feedback()? } else { self.feedback() }; - let count = self.send_count(); + let scope = if self.is_file_review() { SendScope::Pending } else { SendScope::All }; + self.send_feedback_scope(scope) + } + + pub(super) fn resend_all(&mut self) -> Result<()> { + self.send_feedback_scope(SendScope::All) + } + + fn send_feedback_scope(&mut self, scope: SendScope) -> Result<()> { + let mut feedback = self.prepare_feedback(scope)?; + if self.tree.is_some() { + self.folder_counts = std::mem::take(&mut feedback.counts); + } + if feedback.count == 0 { + self.derive_send_state(); + self.status = Some(if scope == SendScope::Pending { + "nothing new to send".into() + } else { + "nothing to send".into() + }); + return Ok(()); + } let target = self.delivery.describe(); - match self.delivery.deliver(&text) { + let retry = if scope == SendScope::All && self.is_file_review() { 'R' } else { 'E' }; + match self.delivery.deliver(&feedback.text) { Ok(()) => { - self.record_delivery(&target)?; - self.archive_submission(&text); - self.send_state = SendState::Sent; - self.status = Some(format!("sent {count} annotation(s) โ†’ {target}")); + let files = feedback.parts.len(); + self.archive_submission(&mut feedback); + let errors = self.remember_delivery(&mut feedback, &target); + self.derive_send_state(); + let verb = if self.delivery.is_agent() { "sent" } else { "copied" }; + let across = + if self.tree.is_some() { format!(" across {files} files") } else { String::new() }; + let mut status = format!("{verb} {} annotation(s){across} โ†’ {target}", feedback.count); + if !errors.is_empty() { + let _ = write!( + status, + "; could not save sent status: {} ยท next send may repeat it", + errors.join("; ") + ); + } + self.status = Some(status); } Err(DeliveryError::Blocked(msg)) => { - self.copy_fallback(&text); - self.status = - Some(format!("{target} is at a dialog โ€” copied to clipboard instead; E retries")); + let copied = if self.copy_fallback(&feedback.text) { " ยท copied to clipboard" } else { "" }; + self.status = Some(format!("{target} is at a dialog{copied} ยท {retry} retry")); self.send_state = SendState::Blocked(msg); } Err(DeliveryError::Unavailable(msg)) => { - self.copy_fallback(&text); - self.status = Some(format!("no agent to send to ({msg}) โ€” copied to clipboard")); + let copied = if self.copy_fallback(&feedback.text) { " ยท copied to clipboard" } else { "" }; + self.status = Some(format!("no agent to send to ({msg}){copied} ยท {retry} retry")); + self.derive_send_state(); } Err(DeliveryError::Failed(err)) => { - self.status = Some(format!("send failed: {err:#}")); + self.status = Some(format!("send failed: {err:#} ยท {retry} retry")); + self.derive_send_state(); } } Ok(()) } - fn copy_fallback(&self, text: &str) { - if self.clipboard { - let _ = Clipboard.deliver(text); + fn copy_fallback(&self, text: &str) -> bool { + self.clipboard && Clipboard.deliver(text).is_ok() + } + + /// Use the stores and ids from the body we delivered, never a fresh export or the + /// whole active set. Attempt every file even when one record cannot be saved. + fn remember_delivery(&mut self, feedback: &mut Feedback, target: &str) -> Vec { + let mut errors = Vec::new(); + for mut part in feedback.parts.drain(..) { + if let Err(err) = part.store.record_delivery(target, &part.ids) { + let name = part + .path + .as_ref() + .map_or_else(|| self.open.source.name.clone(), |p| self.review_file_name(p)); + errors.push(format!("{name}: {err:#}")); + } + if self.tree.is_some() + && let Some(path) = &part.path + { + self.folder_counts.insert(path.clone(), ReviewCounts::for_store(&part.store)); + } + if part.path.as_deref().is_none_or(|p| self.is_open(p)) { + self.open.store = part.store; + } } + errors } - /// Record the submission in the shared feedback archive (contract: Plannotator's - /// `feedback-archive.ts` v1). Never fails the send; the annotation store is the - /// recovery copy when archiving cannot write. - fn archive_submission(&self, feedback: &str) { + /// The shared submission history records only the selected feedback. Finishing a + /// review has its own complete copies in the annotation record and does not rely on it. + fn archive_submission(&self, feedback: &mut Feedback) { use crate::archive::{self, Submission, Target}; if !archive::enabled(|key| std::env::var(key).ok(), &self.data_dir) { return; } let (surface, target, annotations) = if let Some(tree) = &self.tree { - // A folder session submits one body of feedback for the whole session; - // the per-document records are not part of it (contract semantics). ("annotate-folder", Target::file(tree.root()), Vec::new()) } else { - let annotations = Self::annotation_records(&self.open.store); + let annotations = std::mem::take(&mut feedback.annotations); match &self.open.source.provenance { Provenance::File { path } => ("annotate", Target::file(path), annotations), Provenance::AgentMessage { host, session, .. } => ( @@ -89,54 +139,35 @@ impl App { surface, origin, target, - feedback, + feedback: &feedback.text, annotations, - count: self.send_count(), + count: feedback.count, now_ms: None, }); } - fn annotation_records(store: &Store) -> Vec { - store - .placed() - .iter() - .map(|placed| { - let a = placed.annotation; - crate::archive::AnnotationRecord { - id: Some(a.id.clone()), - kind: Some( - match a.anchor.kind() { - Kind::Comment => "comment", - Kind::LooksGood => "looks-good", - Kind::Delete => "delete", - } - .to_owned(), - ), - text: (!a.body.is_empty()).then(|| a.body.clone()), - original_text: (!a.anchor.original_text.is_empty()) - .then(|| a.anchor.original_text.clone()), - } - }) - .collect() - } - - /// Annotations the next send covers: the whole folder's, or the open file's. pub(super) fn send_count(&self) -> usize { - match &self.tree { - Some(tree) => tree.rows.iter().filter(|r| !r.is_dir).map(|r| r.annotations).sum(), - None => self.open.store.placed().len(), - } + if self.is_file_review() { self.review_counts().pending } else { self.open.store.placed().len() } } - /// Text for the Send button. pub(super) fn send_label(&self) -> String { let target = self.delivery.describe(); let count = self.send_count(); + if self.is_file_review() { + let verb = if self.delivery.is_agent() { "Send" } else { "Copy" }; + let across = if self.tree.is_some() { + format!(" across {} files", self.pending_file_count()) + } else { + String::new() + }; + let to = if self.delivery.is_agent() { format!(" โ–ธ {target}") } else { String::new() }; + return format!("{verb} {count} new{across}{to} (E)"); + } if self.delivery.is_agent() { match &self.send_state { SendState::Ready => format!("Send {count} to {target} โ–ธ"), SendState::Sent => format!("Sent โ–ธ {target}"), - SendState::Blocked(_) => format!("{target} at a dialog ยท copied ยท click to retry"), + SendState::Blocked(_) => format!("{target} at a dialog ยท click to retry"), } } else { match &self.send_state { @@ -146,12 +177,12 @@ impl App { } } - /// True when an agent is waiting on feedback that has not been sent since it changed. pub(super) fn has_unsent(&self) -> bool { - self.delivery.is_agent() && self.send_count() > 0 && self.send_state != SendState::Sent + self.delivery.is_agent() + && self.send_count() > 0 + && (self.is_file_review() || self.send_state != SendState::Sent) } - /// Quit, unless an agent is still waiting on feedback: then ask in the footer first. pub(super) fn request_quit(&mut self) { if self.has_unsent() { self.mode = Mode::ConfirmQuit; @@ -160,17 +191,18 @@ impl App { } } - /// Recompute the send state from the record (on load and file switch). pub(super) fn derive_send_state(&mut self) { - let delivered = match &self.tree { - Some(_) => self.folder_all_delivered().unwrap_or(false), - None => self.open.store.all_delivered(), + let delivered = if self.is_file_review() { + let counts = self.review_counts(); + counts.pending == 0 && counts.sent > 0 + } else { + self.open.store.all_delivered() }; self.send_state = if delivered { SendState::Sent } else { SendState::Ready }; } - /// Any annotation change makes the record unsent again. pub(super) fn mark_unsent(&mut self) { - self.send_state = SendState::Ready; + self.update_open_review_counts(); + self.derive_send_state(); } } diff --git a/crates/plannotator-tui/src/store.rs b/crates/plannotator-tui/src/store.rs index 23cef2e..236a452 100644 --- a/crates/plannotator-tui/src/store.rs +++ b/crates/plannotator-tui/src/store.rs @@ -19,7 +19,10 @@ use serde::{Deserialize, Serialize}; use crate::doc::Document; -#[derive(Debug)] +mod review; +use review::timestamp; + +#[derive(Debug, Clone)] pub(crate) struct Store { /// `None` for transient documents: nothing is ever written. path: Option, @@ -29,6 +32,7 @@ pub(crate) struct Store { /// Parallel to `annotations`. resolved: Vec, deliveries: Vec, + archived: Vec, } #[derive(Default, Serialize, Deserialize)] @@ -42,6 +46,10 @@ struct Record { /// Every send, newest last. Lets the UI say "sent" across restarts. #[serde(default, skip_serializing_if = "Vec::is_empty")] deliveries: Vec, + /// Finished annotations, kept whole so restoring preserves ids, anchors and metadata. + /// Additive: consumers of the existing annotations array can ignore this field. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + archived: Vec, } /// One send of the feedback: when, where, and which annotations it covered. @@ -122,6 +130,7 @@ impl Store { annotations: record.annotations, resolved: Vec::new(), deliveries: record.deliveries, + archived: record.archived, }; store.resolve_all(doc); if imported && !store.annotations.is_empty() { @@ -141,6 +150,7 @@ impl Store { annotations: Vec::new(), resolved: Vec::new(), deliveries: Vec::new(), + archived: Vec::new(), } } @@ -150,7 +160,7 @@ impl Store { self.path.is_none() } - /// Every annotated document recorded for `project`, from the records that carry their + /// Every document with active or archived annotations recorded for `project`, from records carrying their /// path (written since 0.5.0). Older records surface through listed tree rows instead. pub(crate) fn annotated_documents(data_dir: &Path, project: &str) -> Vec { let dir = plannotator_tui_schema::annotations_dir(data_dir, project, "x"); @@ -160,7 +170,7 @@ impl Store { .filter_map(Result::ok) .map(|e| e.path().join("annotations.json")) .filter_map(|record| read_record(&record).ok().flatten()) - .filter(|record| !record.annotations.is_empty()) + .filter(|record| !record.annotations.is_empty() || !record.archived.is_empty()) .filter_map(|record| record.path) .collect(); found.sort(); @@ -186,6 +196,7 @@ impl Store { path: self.document.clone(), annotations: self.annotations.clone(), deliveries: self.deliveries.clone(), + archived: self.archived.clone(), }; let json = serde_json::to_string_pretty(&record)?; let tmp = path.with_extension("json.tmp"); @@ -210,7 +221,7 @@ impl Store { version: plannotator_tui_schema::blob_sha(doc.source.as_bytes()), }; let anchor = Anchor::new(rendered, &doc.source, source_range, kind, block); - let now = timestamp(); + let now = timestamp()?; self.annotations.push(Annotation { id: local_id(), document_id: String::new(), @@ -258,15 +269,6 @@ impl Store { true } - /// Replace the body of annotation `id`. - pub(crate) fn edit_body(&mut self, id: &str, body: String) -> Result { - let Some(annotation) = self.annotations.iter_mut().find(|a| a.id == id) else { return Ok(false) }; - annotation.body = body; - annotation.updated_at = timestamp(); - self.save()?; - Ok(true) - } - pub(crate) fn resolve_all(&mut self, doc: &Document) { self.resolved = self .annotations @@ -294,42 +296,11 @@ impl Store { self.annotations.len() } - /// Remember that everything currently recorded was sent to `target`. - pub(crate) fn record_delivery(&mut self, target: &str) -> Result<()> { - self.deliveries.push(Delivered { - at: timestamp(), - target: target.to_owned(), - annotation_ids: self.annotations.iter().map(|a| a.id.clone()).collect(), - }); - self.save() - } - - /// True when the annotations on record are exactly the set of the last send. - pub(crate) fn all_delivered(&self) -> bool { - let Some(last) = self.deliveries.last() else { return false }; - if self.annotations.is_empty() { - return false; - } - let mut sent: Vec<&str> = last.annotation_ids.iter().map(String::as_str).collect(); - let mut have: Vec<&str> = self.annotations.iter().map(|a| a.id.as_str()).collect(); - sent.sort_unstable(); - have.sort_unstable(); - sent == have - } - pub(crate) fn orphans(&self) -> usize { self.resolved.iter().filter(|r| **r == Resolution::Orphan).count() } } -fn timestamp() -> String { - let secs = SystemTime::now().duration_since(UNIX_EPOCH).map_or(0, |d| d.as_secs()); - // RFC 3339 without pulling in a date crate: the API accepts and returns this form. - let (year, month, day) = civil_from_days(secs / 86_400); - let (hour, minute, second) = ((secs / 3600) % 24, (secs / 60) % 60, secs % 60); - format!("{year:04}-{month:02}-{day:02}T{hour:02}:{minute:02}:{second:02}Z") -} - /// Howard Hinnant's days-to-civil, for a dependency-free UTC date. pub(crate) fn civil_from_days(days: u64) -> (u64, u64, u64) { let z = days + 719_468; @@ -408,6 +379,7 @@ mod tests { annotations: Vec::new(), resolved: Vec::new(), deliveries: Vec::new(), + archived: Vec::new(), }; seed.add(&doc, 0..5, "hello".into(), Kind::Comment, "hi".into()).expect("seed sidecar"); let before = std::fs::read_to_string(&sidecar).expect("sidecar"); @@ -442,7 +414,8 @@ mod tests { assert!(!store.all_delivered(), "nothing to send yet"); store.add(&doc, 0..3, "one".into(), Kind::Comment, "a".into()).expect("add"); assert!(!store.all_delivered()); - store.record_delivery("claude in w1:p1").expect("record"); + let ids = store.placed().iter().map(|p| p.annotation.id.clone()).collect::>(); + store.record_delivery("claude in w1:p1", &ids).expect("record"); assert!(store.all_delivered()); let mut reloaded = Store::load(&location, &doc).expect("reload"); diff --git a/crates/plannotator-tui/src/store/review.rs b/crates/plannotator-tui/src/store/review.rs new file mode 100644 index 0000000..7adca2d --- /dev/null +++ b/crates/plannotator-tui/src/store/review.rs @@ -0,0 +1,148 @@ +//! Delivery coverage and recoverable finished reviews. Changes commit to memory only +//! after the record has been saved, so a failed archive or restore leaves it intact. + +use std::collections::HashSet; + +use anyhow::{Context, Result}; +use plannotator_tui_schema::Annotation; +use time::{Duration, OffsetDateTime, format_description::well_known::Rfc3339}; + +use super::{Delivered, Store}; +use crate::doc::Document; + +fn parse_time(value: &str) -> Option { + OffsetDateTime::parse(value, &Rfc3339).ok() +} + +pub(super) fn timestamp() -> Result { + OffsetDateTime::now_utc().format(&Rfc3339).context("formatting annotation time") +} + +impl Store { + fn last_delivery(&self, id: &str) -> Option<&Delivered> { + self.deliveries.iter().rev().find(|d| d.annotation_ids.iter().any(|sent| sent == id)) + } + + /// Coverage belongs to each annotation's last successful send, not the last batch. + /// Unknown timestamps stay pending rather than silently hiding feedback. + pub(crate) fn is_pending(&self, annotation: &Annotation) -> bool { + let covered = self.last_delivery(&annotation.id).and_then(|d| parse_time(&d.at)); + match (parse_time(&annotation.updated_at), covered) { + (Some(updated), Some(sent)) => updated > sent, + _ => true, + } + } + + pub(crate) fn all_delivered(&self) -> bool { + !self.annotations.is_empty() && self.annotations.iter().all(|a| !self.is_pending(a)) + } + + /// Replace a body, advancing the existing timestamp beyond its last send even when + /// the clock has not ticked (or has moved backwards) since that send. + pub(crate) fn edit_body(&mut self, id: &str, body: String) -> Result { + let Some(annotation) = self.annotations.iter().find(|a| a.id == id) else { return Ok(false) }; + if annotation.body == body { + return Ok(false); + } + let previous = + [parse_time(&annotation.updated_at), self.last_delivery(id).and_then(|d| parse_time(&d.at))] + .into_iter() + .flatten() + .max(); + let mut now = OffsetDateTime::now_utc(); + if let Some(previous) = previous { + now = + now.max(previous.checked_add(Duration::nanoseconds(1)).context("advancing annotation time")?); + } + let updated_at = now.format(&Rfc3339).context("formatting annotation time")?; + let mut next = self.clone(); + if let Some(annotation) = next.annotations.iter_mut().find(|a| a.id == id) { + annotation.body = body; + annotation.updated_at = updated_at; + } + next.save()?; + *self = next; + Ok(true) + } + + /// Record only ids that actually appeared in the delivered feedback body. + pub(crate) fn record_delivery(&mut self, target: &str, annotation_ids: &[String]) -> Result<()> { + if annotation_ids.is_empty() { + return Ok(()); + } + let updated = self + .annotations + .iter() + .filter(|a| annotation_ids.contains(&a.id)) + .filter_map(|a| parse_time(&a.updated_at)) + .max(); + let now = updated.map_or_else(OffsetDateTime::now_utc, |at| at.max(OffsetDateTime::now_utc())); + let mut next = self.clone(); + next.deliveries.push(Delivered { + at: now.format(&Rfc3339).context("formatting delivery time")?, + target: target.to_owned(), + annotation_ids: annotation_ids.to_vec(), + }); + next.save()?; + *self = next; + Ok(()) + } + + pub(crate) fn archived(&self) -> &[Annotation] { + &self.archived + } + + /// Finish delivered, unchanged annotations, including ones whose quote is now gone. + /// Pending annotations stay active. The returned ids are the undo operation. + pub(crate) fn archive_sent(&mut self) -> Result> { + let ids: Vec = + self.annotations.iter().filter(|a| !self.is_pending(a)).map(|a| a.id.clone()).collect(); + if ids.is_empty() { + return Ok(ids); + } + let selected: HashSet<&str> = ids.iter().map(String::as_str).collect(); + anyhow::ensure!( + !self.archived.iter().any(|a| selected.contains(a.id.as_str())), + "cannot archive: an annotation with the same id is already archived" + ); + let mut next = self.clone(); + let active = std::mem::take(&mut next.annotations); + let resolved = std::mem::take(&mut next.resolved); + for (annotation, resolution) in active.into_iter().zip(resolved) { + if selected.contains(annotation.id.as_str()) { + next.archived.push(annotation); + } else { + next.annotations.push(annotation); + next.resolved.push(resolution); + } + } + next.save()?; + *self = next; + Ok(ids) + } + + /// Restore without editing timestamps or delivery history. A conflicting active id + /// is left alone, with the archived copy retained for recovery. + pub(crate) fn restore_archived(&mut self, doc: &Document, ids: &[String]) -> Result { + let mut next = self.clone(); + let archived = std::mem::take(&mut next.archived); + let mut restored = 0; + for annotation in archived { + if ids.contains(&annotation.id) && !next.annotations.iter().any(|a| a.id == annotation.id) { + next.annotations.push(annotation); + restored += 1; + } else { + next.archived.push(annotation); + } + } + if restored > 0 { + next.resolve_all(doc); + next.save()?; + *self = next; + } + Ok(restored) + } +} + +#[cfg(test)] +mod tests; diff --git a/crates/plannotator-tui/src/store/review/tests.rs b/crates/plannotator-tui/src/store/review/tests.rs new file mode 100644 index 0000000..5f3f937 --- /dev/null +++ b/crates/plannotator-tui/src/store/review/tests.rs @@ -0,0 +1,180 @@ +#![allow(clippy::expect_used, clippy::indexing_slicing, reason = "tests assert by panicking")] + +use std::collections::BTreeMap; +use std::path::PathBuf; + +use plannotator_tui_schema::{Kind, Reply, State}; + +use super::*; +use crate::store::Location; + +fn fixture(tag: &str) -> (PathBuf, Location, Document, Store) { + let root = std::env::temp_dir().join(format!("plannotator-review-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&root); + let path = root.join("plan.md"); + let location = Location::for_file(&root.join("data"), "project", &path); + let doc = Document::parse("one two three\n".to_owned()); + let store = Store::load(&location, &doc).expect("load empty store"); + (root, location, doc, store) +} + +fn add(store: &mut Store, doc: &Document, quote: &str, body: &str) -> String { + let start = doc.source.find(quote).expect("quote"); + store.add(doc, start..start + quote.len(), quote.into(), Kind::Comment, body.into()).expect("add"); + store.annotations.last().expect("annotation").id.clone() +} + +#[test] +fn incremental_batches_and_edits_remain_correct_after_reopening() { + let (root, location, doc, mut store) = fixture("batches"); + let a = add(&mut store, &doc, "one", "A"); + let b = add(&mut store, &doc, "two", "B"); + store.record_delivery("agent", &[a.clone(), b.clone()]).expect("send A and B"); + let c = add(&mut store, &doc, "three", "C"); + store.record_delivery("agent", &[c]).expect("send C"); + + let mut store = Store::load(&location, &doc).expect("reopen"); + assert!(store.all_delivered(), "sending C must not make A and B pending again"); + store.edit_body(&b, "B revised".into()).expect("edit immediately after send"); + let store = Store::load(&location, &doc).expect("reopen after edit"); + let pending: Vec<&str> = + store.annotations.iter().filter(|a| store.is_pending(a)).map(|a| a.id.as_str()).collect(); + assert_eq!(pending, [b.as_str()]); + assert!(!store.is_pending(&store.annotations[0])); + std::fs::remove_dir_all(root).expect("cleanup"); +} + +#[test] +fn an_edit_advances_past_a_delivery_even_if_the_clock_went_backwards() { + let (_root, _location, doc, _) = fixture("clock"); + let mut store = Store::transient(); + let id = add(&mut store, &doc, "one", "original"); + store.annotations[0].updated_at = "2099-01-01T00:00:00Z".into(); + store.deliveries.push(Delivered { + at: "2099-01-01T00:00:00Z".into(), + target: "agent".into(), + annotation_ids: vec![id.clone()], + }); + assert!(!store.is_pending(&store.annotations[0])); + assert!(!store.edit_body(&id, "original".into()).expect("unchanged body")); + assert!(!store.is_pending(&store.annotations[0])); + store.edit_body(&id, "edited".into()).expect("edit"); + assert!(store.is_pending(&store.annotations[0])); + store.record_delivery("agent", &[id]).expect("send despite clock skew"); + assert!(!store.is_pending(&store.annotations[0])); +} + +#[test] +fn rfc3339_offsets_and_precision_are_compared_as_instants() { + let doc = Document::parse("one\n".into()); + let mut store = Store::transient(); + let id = add(&mut store, &doc, "one", "note"); + for (updated, sent, pending) in [ + ("2026-09-09T05:00:00Z", "2026-09-09T05:00:00.000Z", false), + ("2026-09-09T05:00:00.001Z", "2026-09-09T05:00:00Z", true), + ("2026-09-09T13:00:00+08:00", "2026-09-09T05:00:00Z", false), + ("2026-09-09T00:00:00-05:00", "2026-09-09T05:00:00Z", false), + ("unknown", "2026-09-09T05:00:00Z", true), + ("2026-09-09T05:00:00Z", "unknown", true), + ] { + store.annotations[0].updated_at = updated.into(); + store.deliveries = + vec![Delivered { at: sent.into(), target: "agent".into(), annotation_ids: vec![id.clone()] }]; + assert_eq!(store.is_pending(&store.annotations[0]), pending, "{updated} vs {sent}"); + } +} + +#[test] +fn archiving_keeps_pending_notes_and_restores_complete_annotations_and_history() { + let (root, location, doc, mut store) = fixture("restore"); + let a = add(&mut store, &doc, "one", "A"); + let b = add(&mut store, &doc, "two", "B"); + let annotation = &mut store.annotations[0]; + annotation.state = State::Resolved; + annotation.author = Some("reviewer".into()); + annotation.attachments = vec!["attachment.png".into()]; + annotation.other.insert("future_field".into(), serde_json::json!({"keep": true})); + annotation.replies.push(Reply { + id: "reply-1".into(), + annotation_id: a.clone(), + body: "keep this reply".into(), + author: None, + author_name: None, + created_at: annotation.created_at.clone(), + updated_at: annotation.updated_at.clone(), + other: BTreeMap::default(), + }); + let original = annotation.clone(); + store.record_delivery("agent", &[a.clone(), b.clone()]).expect("send"); + store.edit_body(&b, "B edited".into()).expect("edit B"); + add(&mut store, &doc, "three", "C"); + let history = store.deliveries.clone(); + + assert_eq!(store.archive_sent().expect("archive").as_slice(), std::slice::from_ref(&a)); + assert_eq!(store.len(), 2, "B and C remain active"); + let mut store = Store::load(&location, &doc).expect("reopen archive"); + assert_eq!(store.archived(), std::slice::from_ref(&original)); + assert_eq!(store.deliveries, history); + assert_eq!(store.restore_archived(&doc, std::slice::from_ref(&a)).expect("restore"), 1); + let store = Store::load(&location, &doc).expect("reopen restored"); + let restored = store.annotations.iter().find(|a| a.id == original.id).expect("same id"); + assert_eq!(restored, &original); + assert!(!store.is_pending(restored), "restoring must not resend it"); + assert!(store.archived().is_empty()); + assert_eq!(store.deliveries, history); + std::fs::remove_dir_all(root).expect("cleanup"); +} + +#[test] +fn archive_and_restore_failures_leave_memory_and_disk_intact() { + let (root, location, doc, mut store) = fixture("write-failure"); + let id = add(&mut store, &doc, "one", "keep me"); + store.record_delivery("agent", std::slice::from_ref(&id)).expect("send"); + let before = std::fs::read(&location.record).expect("record"); + let blocked_tmp = location.record.with_extension("json.tmp"); + std::fs::create_dir(&blocked_tmp).expect("block temporary file"); + assert!(store.archive_sent().is_err()); + assert_eq!(store.len(), 1); + assert!(store.archived().is_empty()); + assert_eq!(std::fs::read(&location.record).expect("record"), before); + std::fs::remove_dir(&blocked_tmp).expect("unblock"); + + store.archive_sent().expect("archive"); + let before = std::fs::read(&location.record).expect("archived record"); + std::fs::create_dir(&blocked_tmp).expect("block temporary file"); + assert!(store.restore_archived(&doc, &[id]).is_err()); + assert_eq!(store.len(), 0); + assert_eq!(store.archived().len(), 1); + assert_eq!(std::fs::read(&location.record).expect("record"), before); + std::fs::remove_dir_all(root).expect("cleanup"); +} + +#[test] +fn restoring_never_overwrites_an_active_annotation_with_the_same_id() { + let doc = Document::parse("one\n".into()); + let mut store = Store::transient(); + let id = add(&mut store, &doc, "one", "newer body"); + let mut archived = store.annotations[0].clone(); + archived.body = "older body".into(); + store.archived.push(archived.clone()); + assert_eq!(store.restore_archived(&doc, &[id]).expect("restore"), 0); + assert_eq!(store.annotations[0].body, "newer body"); + assert_eq!(store.archived(), [archived]); +} + +#[test] +fn archive_only_records_are_discoverable_and_sent_orphans_can_be_finished() { + let (root, location, doc, mut store) = fixture("orphan"); + let id = add(&mut store, &doc, "one", "remove this"); + store.record_delivery("agent", std::slice::from_ref(&id)).expect("send"); + let changed = Document::parse("different text\n".into()); + store.resolve_all(&changed); + assert_eq!(store.orphans(), 1); + assert_eq!(store.archive_sent().expect("finish").as_slice(), std::slice::from_ref(&id)); + assert_eq!(Store::annotated_documents(&root.join("data"), "project"), [root.join("plan.md")]); + let mut store = Store::load(&location, &changed).expect("reopen"); + store.restore_archived(&changed, &[id]).expect("restore orphan"); + assert_eq!(store.orphans(), 1); + assert!(store.all_delivered()); + std::fs::remove_dir_all(root).expect("cleanup"); +} From d4f2b3653b5299f642d9e8f133d9f5327455580b Mon Sep 17 00:00:00 2001 From: yunyaozhou Date: Wed, 9 Sep 2026 22:49:47 +0800 Subject: [PATCH 02/10] fix(review): preserve notes changed during delivery --- crates/plannotator-tui/src/app/send.rs | 31 ++++++- crates/plannotator-tui/src/app/send/tests.rs | 97 ++++++++++++++++++++ crates/plannotator-tui/src/store/review.rs | 7 ++ 3 files changed, 131 insertions(+), 4 deletions(-) create mode 100644 crates/plannotator-tui/src/app/send/tests.rs diff --git a/crates/plannotator-tui/src/app/send.rs b/crates/plannotator-tui/src/app/send.rs index a8f19ac..72cfc4c 100644 --- a/crates/plannotator-tui/src/app/send.rs +++ b/crates/plannotator-tui/src/app/send.rs @@ -5,9 +5,13 @@ use std::fmt::Write as _; use anyhow::Result; use plannotator_tui_schema::Provenance; -use super::feedback::{Feedback, ReviewCounts, SendScope}; +use super::feedback::{Feedback, FeedbackPart, ReviewCounts, SendScope}; use super::{App, Mode}; use crate::delivery::{Clipboard, Delivery as _, DeliveryError}; +use crate::store::{Location, Store}; + +#[cfg(test)] +mod tests; #[derive(Debug, Clone, PartialEq, Eq)] pub(super) enum SendState { @@ -83,12 +87,12 @@ impl App { self.clipboard && Clipboard.deliver(text).is_ok() } - /// Use the stores and ids from the body we delivered, never a fresh export or the - /// whole active set. Attempt every file even when one record cannot be saved. + /// Keep the ids from the body we delivered. Attempt every file even when one record + /// cannot be saved, and refresh counts from any intervening changes. fn remember_delivery(&mut self, feedback: &mut Feedback, target: &str) -> Vec { let mut errors = Vec::new(); for mut part in feedback.parts.drain(..) { - if let Err(err) = part.store.record_delivery(target, &part.ids) { + if let Err(err) = self.record_feedback_delivery(&mut part, target) { let name = part .path .as_ref() @@ -107,6 +111,25 @@ impl App { errors } + fn record_feedback_delivery(&self, part: &mut FeedbackPart, target: &str) -> Result<()> { + if self.is_file_review() + && let Some(path) = &part.path + { + let latest = if self.is_open(path) { + let location = Location::for_file(&self.data_dir, &self.project, path); + Store::load(&location, &self.open.doc)? + } else { + self.load_review_file(path)?.1 + }; + let unchanged = part.store.same_review(&latest); + part.store = latest; + // The transport can block while another writer changes the shared record. + // Keep that record intact; its newer notes were not in the delivered body. + anyhow::ensure!(unchanged, "annotations changed while sending; kept the newer record"); + } + part.store.record_delivery(target, &part.ids) + } + /// The shared submission history records only the selected feedback. Finishing a /// review has its own complete copies in the annotation record and does not rely on it. fn archive_submission(&self, feedback: &mut Feedback) { diff --git a/crates/plannotator-tui/src/app/send/tests.rs b/crates/plannotator-tui/src/app/send/tests.rs new file mode 100644 index 0000000..a93d9ec --- /dev/null +++ b/crates/plannotator-tui/src/app/send/tests.rs @@ -0,0 +1,97 @@ +#![allow(clippy::expect_used, clippy::indexing_slicing, reason = "tests assert by panicking")] + +use std::cell::RefCell; +use std::rc::Rc; + +use plannotator_tui_schema::Kind; + +use crate::app::review_test_support::{RecordingDelivery, file_app, folder_app, press}; +use crate::delivery::{Delivery, DeliveryError}; +use crate::doc::Document; +use crate::store::{Location, Store}; + +struct EditingDelivery { + transport: RecordingDelivery, + location: Location, + document: Document, + annotation_id: String, + saved: Rc>>, +} + +impl Delivery for EditingDelivery { + fn describe(&self) -> String { + self.transport.describe() + } + + fn is_agent(&self) -> bool { + true + } + + fn deliver(&self, feedback: &str) -> Result<(), DeliveryError> { + let mut store = Store::load(&self.location, &self.document).expect("second writer"); + store.edit_body(&self.annotation_id, "edited during send".into()).expect("edit"); + let start = self.document.source.find("two").expect("quote"); + store + .add(&self.document, start..start + 3, "two".into(), Kind::Comment, "added during send".into()) + .expect("add"); + *self.saved.borrow_mut() = std::fs::read(&self.location.record).expect("newer record"); + self.transport.deliver(feedback) + } +} + +#[test] +fn send_writeback_preserves_changes_made_while_the_transport_is_running() { + for folder in [false, true] { + let (root, mut app, delivery) = + if folder { folder_app("concurrent-folder") } else { file_app("concurrent-file") }; + let path = if folder { + app.add_quote_annotation("one", Kind::Comment, "unchanged open file".into()).expect("A"); + let path = root.join("docs/b.md"); + std::fs::write(&path, "one\n\ntwo\n").expect("unopened file"); + let (doc, mut store) = app.load_review_file(&path).expect("B store"); + store.add(&doc, 0..3, "one".into(), Kind::Comment, "original note".into()).expect("B"); + path + } else { + app.add_quote_annotation("one", Kind::Comment, "original note".into()).expect("note"); + root.join("docs/a.md") + }; + let (document, store) = app.load_review_file(&path).expect("review"); + let id = store.placed()[0].annotation.id.clone(); + let location = Location::for_file(&app.data_dir, &app.project, &path); + let saved = Rc::new(RefCell::new(Vec::new())); + app.delivery = Box::new(EditingDelivery { + transport: delivery.clone(), + location: location.clone(), + document, + annotation_id: id, + saved: saved.clone(), + }); + + press(&mut app, 'E'); + + let (_, store) = app.load_review_file(&path).expect("reload record"); + assert_eq!(store.len(), 2, "the note added during delivery must survive"); + assert_eq!(std::fs::read(&location.record).expect("record"), *saved.borrow()); + assert!(store.placed().iter().all(|p| store.is_pending(p.annotation))); + assert_eq!(app.send_count(), 2, "new and edited notes stay pending in the UI"); + let status = app.status.as_deref().expect("status"); + assert!(status.contains("changed while sending"), "{status}"); + assert!(status.contains("next send may repeat it"), "{status}"); + assert!(status.contains(if folder { "b.md" } else { "a.md" }), "{status}"); + assert_eq!(delivery.calls.borrow().len(), 1); + let sent = delivery.calls.borrow()[0].clone(); + assert!(sent.contains("original note")); + assert!(!sent.contains("during send")); + if folder { + assert!(app.open.store.all_delivered(), "unchanged files still record the send"); + } + + app.delivery = Box::new(delivery.clone()); + press(&mut app, 'E'); + assert_eq!(app.send_count(), 0, "the newer notes can be sent without reopening"); + let resent = delivery.calls.borrow()[1].clone(); + assert!(resent.contains("edited during send") && resent.contains("added during send")); + assert!(!resent.contains("unchanged open file")); + std::fs::remove_dir_all(root).expect("cleanup"); + } +} diff --git a/crates/plannotator-tui/src/store/review.rs b/crates/plannotator-tui/src/store/review.rs index 7adca2d..76d11f1 100644 --- a/crates/plannotator-tui/src/store/review.rs +++ b/crates/plannotator-tui/src/store/review.rs @@ -19,6 +19,13 @@ pub(super) fn timestamp() -> Result { } impl Store { + /// Compare persisted review data, independent of resolution against the document. + pub(crate) fn same_review(&self, other: &Self) -> bool { + self.annotations == other.annotations + && self.deliveries == other.deliveries + && self.archived == other.archived + } + fn last_delivery(&self, id: &str) -> Option<&Delivered> { self.deliveries.iter().rev().find(|d| d.annotation_ids.iter().any(|sent| sent == id)) } From bb8aadfaf5fdd15c46ce4f79ef57e55bd86cb4b0 Mon Sep 17 00:00:00 2001 From: yunyaozhou Date: Wed, 9 Sep 2026 23:03:55 +0800 Subject: [PATCH 03/10] fix(review): name the file in restore conflicts --- crates/plannotator-tui/src/app/review.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/plannotator-tui/src/app/review.rs b/crates/plannotator-tui/src/app/review.rs index 364b084..f013c66 100644 --- a/crates/plannotator-tui/src/app/review.rs +++ b/crates/plannotator-tui/src/app/review.rs @@ -148,7 +148,10 @@ impl App { let Some(item) = self.archive_items.get(self.archive_cursor).cloned() else { return }; match self.restore_review_file(&item.path, std::slice::from_ref(&item.annotation.id)) { Ok(0) => { - self.status = Some("could not restore: an annotation with this id is already active".into()); + self.status = Some(format!( + "could not restore {}: an annotation with this id is already active", + self.review_file_name(&item.path) + )); } Ok(_) => { for batch in &mut self.undo_archive { From d0398397bac23fce31bb3c01e3a5c44b000c5c1a Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Wed, 9 Sep 2026 08:48:00 -0700 Subject: [PATCH 04/10] fix(review): write timestamps with fixed millisecond precision --- crates/plannotator-tui/src/store/review.rs | 42 +++++++++++++--- .../plannotator-tui/src/store/review/tests.rs | 48 +++++++++++++++++++ 2 files changed, 83 insertions(+), 7 deletions(-) diff --git a/crates/plannotator-tui/src/store/review.rs b/crates/plannotator-tui/src/store/review.rs index 76d11f1..e3e28cc 100644 --- a/crates/plannotator-tui/src/store/review.rs +++ b/crates/plannotator-tui/src/store/review.rs @@ -5,17 +5,44 @@ use std::collections::HashSet; use anyhow::{Context, Result}; use plannotator_tui_schema::Annotation; -use time::{Duration, OffsetDateTime, format_description::well_known::Rfc3339}; +use time::{Duration, OffsetDateTime, UtcOffset, format_description::well_known::Rfc3339}; use super::{Delivered, Store}; use crate::doc::Document; +/// Parsing stays lenient: any RFC 3339 offset and sub-second precision is accepted. fn parse_time(value: &str) -> Option { OffsetDateTime::parse(value, &Rfc3339).ok() } +/// Every timestamp the record writes has one shape, `YYYY-MM-DDTHH:MM:SS.mmmZ`, so a +/// record never mixes precisions (see `archive::iso_millis`). Finer digits are dropped. +fn format_millis(at: OffsetDateTime) -> Result { + let at = at.checked_to_offset(UtcOffset::UTC).context("annotation time out of range")?; + Ok(format!( + "{:04}-{:02}-{:02}T{:02}:{:02}:{:02}.{:03}Z", + at.year(), + u8::from(at.month()), + at.day(), + at.hour(), + at.minute(), + at.second(), + at.millisecond() + )) +} + +/// The millisecond at or after `at`, so a stored copy never sorts before the instant it +/// stands for when that instant carried finer digits. +fn ceil_millis(at: OffsetDateTime) -> Result { + let below = i64::from(at.nanosecond() % 1_000_000); + if below == 0 { + return Ok(at); + } + at.checked_add(Duration::nanoseconds(1_000_000 - below)).context("advancing annotation time") +} + pub(super) fn timestamp() -> Result { - OffsetDateTime::now_utc().format(&Rfc3339).context("formatting annotation time") + format_millis(OffsetDateTime::now_utc()) } impl Store { @@ -45,7 +72,8 @@ impl Store { } /// Replace a body, advancing the existing timestamp beyond its last send even when - /// the clock has not ticked (or has moved backwards) since that send. + /// the clock has not ticked (or has moved backwards) since that send. One millisecond + /// is the smallest step the stored shape can represent. pub(crate) fn edit_body(&mut self, id: &str, body: String) -> Result { let Some(annotation) = self.annotations.iter().find(|a| a.id == id) else { return Ok(false) }; if annotation.body == body { @@ -58,10 +86,10 @@ impl Store { .max(); let mut now = OffsetDateTime::now_utc(); if let Some(previous) = previous { - now = - now.max(previous.checked_add(Duration::nanoseconds(1)).context("advancing annotation time")?); + now = now + .max(previous.checked_add(Duration::milliseconds(1)).context("advancing annotation time")?); } - let updated_at = now.format(&Rfc3339).context("formatting annotation time")?; + let updated_at = format_millis(now)?; let mut next = self.clone(); if let Some(annotation) = next.annotations.iter_mut().find(|a| a.id == id) { annotation.body = body; @@ -86,7 +114,7 @@ impl Store { let now = updated.map_or_else(OffsetDateTime::now_utc, |at| at.max(OffsetDateTime::now_utc())); let mut next = self.clone(); next.deliveries.push(Delivered { - at: now.format(&Rfc3339).context("formatting delivery time")?, + at: format_millis(ceil_millis(now)?)?, target: target.to_owned(), annotation_ids: annotation_ids.to_vec(), }); diff --git a/crates/plannotator-tui/src/store/review/tests.rs b/crates/plannotator-tui/src/store/review/tests.rs index 5f3f937..40d642d 100644 --- a/crates/plannotator-tui/src/store/review/tests.rs +++ b/crates/plannotator-tui/src/store/review/tests.rs @@ -64,6 +64,54 @@ fn an_edit_advances_past_a_delivery_even_if_the_clock_went_backwards() { assert!(!store.is_pending(&store.annotations[0])); } +/// `YYYY-MM-DDTHH:MM:SS.mmmZ`: exactly three fractional digits and a trailing `Z`. +fn has_millis_shape(value: &str) -> bool { + let bytes = value.as_bytes(); + bytes.len() == 24 + && bytes[19] == b'.' + && bytes[23] == b'Z' + && bytes[20..23].iter().all(u8::is_ascii_digit) + && parse_time(value).is_some() +} + +#[test] +fn every_written_timestamp_has_three_fractional_digits_and_a_trailing_z() { + let doc = Document::parse("one\n".into()); + let mut store = Store::transient(); + let id = add(&mut store, &doc, "one", "note"); + let fresh = store.annotations[0].updated_at.clone(); + assert!(has_millis_shape(&fresh), "fresh timestamp was {fresh:?}"); + assert_eq!(store.annotations[0].created_at, fresh); + + store.record_delivery("agent", std::slice::from_ref(&id)).expect("send"); + let sent = store.deliveries[0].at.clone(); + assert!(has_millis_shape(&sent), "delivery timestamp was {sent:?}"); + assert!(!store.is_pending(&store.annotations[0])); + + store.edit_body(&id, "edited".into()).expect("edit right after the send"); + let advanced = store.annotations[0].updated_at.clone(); + assert!(has_millis_shape(&advanced), "advanced timestamp was {advanced:?}"); + assert!(store.is_pending(&store.annotations[0])); + + // A clock that has not moved (or went backwards) since the send advances the edit by + // one millisecond, the smallest step the stored shape can represent. + store.annotations[0].updated_at = "2099-01-01T00:00:00Z".into(); + store.deliveries = vec![Delivered { + at: "2099-01-01T00:00:00.000Z".into(), + target: "agent".into(), + annotation_ids: vec![id.clone()], + }]; + store.edit_body(&id, "edited again".into()).expect("edit against a stalled clock"); + assert_eq!(store.annotations[0].updated_at, "2099-01-01T00:00:00.001Z"); + + // Finer digits in an older record are read but never written back: a delivery after + // such a timestamp rounds up so the annotation does not stay pending. + store.annotations[0].updated_at = "2099-01-01T00:00:00.425677Z".into(); + store.record_delivery("agent", &[id]).expect("send after a fine-grained timestamp"); + assert_eq!(store.deliveries.last().expect("delivery").at, "2099-01-01T00:00:00.426Z"); + assert!(!store.is_pending(&store.annotations[0])); +} + #[test] fn rfc3339_offsets_and_precision_are_compared_as_instants() { let doc = Document::parse("one\n".into()); From 9bf2173d258ba8157022736cb87c91a17d24098c Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Wed, 9 Sep 2026 08:49:09 -0700 Subject: [PATCH 05/10] test(review): pin that removing a sent annotation keeps the review sent --- crates/plannotator-tui/src/app/send/tests.rs | 50 +++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/crates/plannotator-tui/src/app/send/tests.rs b/crates/plannotator-tui/src/app/send/tests.rs index a93d9ec..f478385 100644 --- a/crates/plannotator-tui/src/app/send/tests.rs +++ b/crates/plannotator-tui/src/app/send/tests.rs @@ -3,9 +3,11 @@ use std::cell::RefCell; use std::rc::Rc; -use plannotator_tui_schema::Kind; +use plannotator_tui_schema::{DocumentSource, Kind, Provenance}; use crate::app::review_test_support::{RecordingDelivery, file_app, folder_app, press}; +use crate::app::send::SendState; +use crate::app::{Focus, Mode, Open}; use crate::delivery::{Delivery, DeliveryError}; use crate::doc::Document; use crate::store::{Location, Store}; @@ -95,3 +97,49 @@ fn send_writeback_preserves_changes_made_while_the_transport_is_running() { std::fs::remove_dir_all(root).expect("cleanup"); } } + +/// The rule: removing a delivered note creates nothing new to send. After A and B were +/// sent and B is removed, the review stays `Sent` and `q` quits without asking. This +/// holds for file reviews and reply reviews alike; 0.7.0 re-armed a reply review to +/// "Send 1" here, which would have resent A although it was never changed. +#[test] +fn removing_a_sent_annotation_leaves_the_review_sent() { + for reply in [false, true] { + let (root, mut app, delivery) = file_app("remove-after-send"); + if reply { + let source = DocumentSource::new( + "one\n\ntwo\n".into(), + "agent reply", + true, + Provenance::AgentMessage { + host: "claude".into(), + session: None, + message_id: Some("message-1".into()), + }, + ); + app.open = Open::new(source, 80, &app.data_dir, &app.project).expect("reply"); + } + app.add_quote_annotation("one", Kind::Comment, "A".into()).expect("A"); + app.add_quote_annotation("two", Kind::Comment, "B".into()).expect("B"); + assert!(app.has_unsent(), "reply={reply}"); + press(&mut app, 'E'); + assert_eq!(delivery.calls.borrow().len(), 1, "reply={reply}"); + assert_eq!(app.send_state, SendState::Sent, "reply={reply}"); + + app.focus = Focus::Rail; + app.rail_cursor = 1; + press(&mut app, 'x'); + assert_eq!(app.open.store.len(), 1, "reply={reply}"); + assert_eq!(app.status.as_deref(), Some("annotation removed"), "reply={reply}"); + assert_eq!(app.send_state, SendState::Sent, "reply={reply}: removing B is not a change to send"); + assert!(!app.has_unsent(), "reply={reply}"); + if !reply { + assert_eq!(app.send_count(), 0, "A was delivered and is still unchanged"); + } + press(&mut app, 'q'); + assert!(app.quit, "reply={reply}: nothing to send, so no confirmation"); + assert_eq!(app.mode, Mode::Browse, "reply={reply}"); + assert_eq!(delivery.calls.borrow().len(), 1, "reply={reply}: no send was triggered"); + std::fs::remove_dir_all(root).expect("cleanup"); + } +} From 66cd1655492729651f73d4662e4688f0ddc680ff Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Wed, 9 Sep 2026 08:54:39 -0700 Subject: [PATCH 06/10] fix(tui): keep the file name, counters and key help beside a footer status --- crates/plannotator-tui/src/app/draw.rs | 10 +++++----- crates/plannotator-tui/src/app/tests.rs | 24 ++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/crates/plannotator-tui/src/app/draw.rs b/crates/plannotator-tui/src/app/draw.rs index fff93bb..79921a1 100644 --- a/crates/plannotator-tui/src/app/draw.rs +++ b/crates/plannotator-tui/src/app/draw.rs @@ -389,10 +389,6 @@ impl App { self.geometry.undo_button = Some(rect); area.width = area.width.saturating_sub(width); } - if let Some(status) = &self.status { - frame.render_widget(Paragraph::new(format!(" {status}")), area); - return; - } let orphans = self.open.store.orphans(); // The status leads: it is the transient half of the line, and the name and counters // it pushes right are on screen for the whole session anyway. @@ -421,8 +417,12 @@ impl App { Focus::Rail => "j/k ยท e edit ยท x remove ยท tab ยท q quit ", Focus::Document => "drag or v select ยท c comment ยท E send ยท tab ยท q quit ", }; + // The status must stay readable at any width, so the key help yields columns to it + // (and is clipped) rather than the other way round. + let status_width = self.status.as_ref().map_or(0, |s| s.width() + 1) as u16; + let help_width = (help.width() as u16).min(area.width.saturating_sub(status_width.max(10))); let [left_area, right_area] = - Layout::horizontal([Constraint::Min(10), Constraint::Length(help.width() as u16)]).areas(area); + Layout::horizontal([Constraint::Min(10), Constraint::Length(help_width)]).areas(area); frame.render_widget( Paragraph::new(Line::from(Span::raw(format!(" {}", parts.join(" ยท "))).dim())), left_area, diff --git a/crates/plannotator-tui/src/app/tests.rs b/crates/plannotator-tui/src/app/tests.rs index 1f4d3fa..324630d 100644 --- a/crates/plannotator-tui/src/app/tests.rs +++ b/crates/plannotator-tui/src/app/tests.rs @@ -189,6 +189,30 @@ fn a_status_leads_the_footer_so_a_narrow_pane_cannot_truncate_it_away() { let rows = draw(&mut app); let footer = row(&rows, rows.len() - 1); assert!(footer.trim_start().starts_with("no session id from Herdr"), "footer was {footer:?}"); + // At 80 columns the whole status is still there: the key help gives up its columns. + assert!( + footer.contains("no session id from Herdr, showing the newest transcript for this folder"), + "footer was {footer:?}" + ); +} + +#[test] +fn a_status_leads_the_footer_and_the_name_counters_and_key_help_stay() { + let mut app = app(Box::new(Discard)); + app.add_block_annotation(0, Kind::Comment, "x".to_owned()).expect("annotation"); + app.set_status("comment saved".to_owned()); + let rows = draw_sized(&mut app, 140, 20); + let footer = row(&rows, 19); + assert!( + footer.trim_start().starts_with("comment saved ยท plan.md ยท 1 annotations ยท block 1/"), + "footer was {footer:?}" + ); + assert!(footer.trim_end().ends_with("ยท q quit"), "key help is still drawn: {footer:?}"); + // Once the status is cleared the same line continues with the name. + app.status = None; + let rows = draw_sized(&mut app, 140, 20); + let footer = row(&rows, 19); + assert!(footer.trim_start().starts_with("plan.md ยท 1 annotations ยท block 1/"), "footer was {footer:?}"); } #[test] From 29fb29e2f715d64883e2c043bfce7b3a125a9fc8 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Wed, 9 Sep 2026 09:06:42 -0700 Subject: [PATCH 07/10] fix(feedback): keep the trailing newline on folder feedback --- crates/plannotator-tui/src/app/feedback.rs | 5 ++++ .../plannotator-tui/src/app/feedback/tests.rs | 29 +++++++++++++++++++ 2 files changed, 34 insertions(+) diff --git a/crates/plannotator-tui/src/app/feedback.rs b/crates/plannotator-tui/src/app/feedback.rs index 0fa0454..bd76227 100644 --- a/crates/plannotator-tui/src/app/feedback.rs +++ b/crates/plannotator-tui/src/app/feedback.rs @@ -228,6 +228,11 @@ impl App { feedback.add(Some(path), &name, &doc, store, scope); } } + // Folder feedback has always ended each file's block with one extra newline, so + // the body and `--export ` keep the shape earlier releases produced. + if !feedback.text.is_empty() { + feedback.text.push('\n'); + } Ok(feedback) } diff --git a/crates/plannotator-tui/src/app/feedback/tests.rs b/crates/plannotator-tui/src/app/feedback/tests.rs index a048d9b..7324f7f 100644 --- a/crates/plannotator-tui/src/app/feedback/tests.rs +++ b/crates/plannotator-tui/src/app/feedback/tests.rs @@ -159,6 +159,35 @@ fn folder_counts_and_delivery_cover_collapsed_files_but_exclude_orphans_and_sibl std::fs::remove_dir_all(root).expect("cleanup"); } +/// Folder output is `writeln!` per file: every block, including the last, is followed by +/// a blank line, while a single file's export ends with the block's own newline only. +#[test] +fn folder_feedback_ends_every_file_block_with_a_blank_line() { + let (root, mut app, delivery) = folder_app("folder-newline"); + app.add_quote_annotation("one", Kind::Comment, "note A".into()).expect("A"); + let other = root.join("docs/b.md"); + std::fs::write(&other, "beta\n").expect("second file"); + let (doc, mut store) = app.load_review_file(&other).expect("second store"); + store.add(&doc, 0..4, "beta".into(), Kind::Comment, "note B".into()).expect("B"); + app.refresh_review_counts().expect("counts"); + + let single = app.feedback(); + assert!(single.ends_with("> note A\n\n") && !single.ends_with("\n\n\n"), "{single:?}"); + let folder = app.folder_feedback().expect("folder export"); + assert_eq!( + folder, + format!( + "{single}\n# Annotations on b.md\n\n## Annotation 1 (line 1)\nComment on: \"beta\"\n> note B\n\n\n" + ) + ); + assert!(folder.ends_with("> note B\n\n\n"), "{folder:?}"); + + press(&mut app, 'E'); + let sent = delivery.calls.borrow()[0].clone(); + assert_eq!(sent, folder, "the send body is the export"); + std::fs::remove_dir_all(root).expect("cleanup"); +} + #[test] fn reply_reviews_keep_sending_the_whole_transient_review() { let (root, mut app, delivery) = file_app("reply-scope"); From 2f401a5d3591a5ac7cece4271596c24a75ae4ba7 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Wed, 9 Sep 2026 09:10:53 -0700 Subject: [PATCH 08/10] fix(folder): skip unreadable annotated files instead of failing to open --- crates/plannotator-tui/src/app/feedback.rs | 37 ++++++-- .../plannotator-tui/src/app/feedback/tests.rs | 84 ++++++++++++++++++- crates/plannotator-tui/src/app/mod.rs | 15 +++- .../plannotator-tui/src/app/review/tests.rs | 6 +- .../src/app/review_test_support.rs | 4 +- crates/plannotator-tui/src/store/review.rs | 13 ++- .../plannotator-tui/src/store/review/tests.rs | 27 +++++- 7 files changed, 163 insertions(+), 23 deletions(-) diff --git a/crates/plannotator-tui/src/app/feedback.rs b/crates/plannotator-tui/src/app/feedback.rs index bd76227..f89212c 100644 --- a/crates/plannotator-tui/src/app/feedback.rs +++ b/crates/plannotator-tui/src/app/feedback.rs @@ -170,22 +170,49 @@ impl App { Ok((doc, store)) } - pub(super) fn refresh_review_counts(&mut self) -> Result<()> { + /// Count what can be read. A file whose document or record cannot be loaded (permission + /// denied, a corrupt record) is left out and remembered; it never stops the folder from + /// opening, reloading or expanding. The status names the skipped files once, when the + /// set changes; sending still reports such a file as an error. + pub(super) fn refresh_review_counts(&mut self) { if self.tree.is_none() { - return Ok(()); + return; } let mut counts = HashMap::new(); + let mut unreadable = Vec::new(); for path in self.review_files() { let count = if self.is_open(&path) && path.is_file() { ReviewCounts::for_store(&self.open.store) - } else { - let (_, store) = self.load_review_file(&path)?; + } else if let Ok((_, store)) = self.load_review_file(&path) { ReviewCounts::for_store(&store) + } else { + unreadable.push(path); + continue; }; counts.insert(path, count); } self.folder_counts = counts; - Ok(()) + if unreadable != self.unreadable_files { + self.unreadable_files = unreadable; + if let Some(note) = self.unreadable_note() { + self.status = Some(note); + } + } + } + + /// One line naming the files the counts had to skip. + pub(super) fn unreadable_note(&self) -> Option { + if self.unreadable_files.is_empty() { + return None; + } + let names: Vec = + self.unreadable_files.iter().take(3).map(|path| self.review_file_name(path)).collect(); + let more = if self.unreadable_files.len() > names.len() { ", ..." } else { "" }; + Some(format!( + "skipped {} unreadable file(s): {}{more}", + self.unreadable_files.len(), + names.join(", ") + )) } pub(super) fn update_open_review_counts(&mut self) { diff --git a/crates/plannotator-tui/src/app/feedback/tests.rs b/crates/plannotator-tui/src/app/feedback/tests.rs index 7324f7f..8a79864 100644 --- a/crates/plannotator-tui/src/app/feedback/tests.rs +++ b/crates/plannotator-tui/src/app/feedback/tests.rs @@ -1,10 +1,15 @@ #![allow(clippy::expect_used, clippy::indexing_slicing, reason = "tests assert by panicking")] +use std::path::Path; + use plannotator_tui_schema::{DocumentSource, Kind, Provenance}; +use ratatui::crossterm::event::{Event, KeyCode, KeyEvent}; use crate::app::review_test_support::{Outcome, click, draw, file_app, folder_app, press, reopen}; -use crate::app::{Mode, Open}; -use crate::store::Location; +use crate::app::{App, Focus, Mode, Open}; +use crate::doc::Document; +use crate::store::{Location, Store}; +use crate::tree::Tree; #[test] fn incremental_send_and_explicit_resend_use_the_same_set_for_body_history_and_ids() { @@ -133,7 +138,7 @@ fn folder_counts_and_delivery_cover_collapsed_files_but_exclude_orphans_and_sibl store.record_delivery("test agent", &[id]).expect("sent"); store.archive_sent().expect("archive"); - app.refresh_review_counts().expect("refresh counts"); + app.refresh_review_counts(); assert_eq!(app.send_count(), 2); assert_eq!(app.pending_file_count(), 2); assert_eq!(app.review_counts().archived, 1); @@ -169,7 +174,7 @@ fn folder_feedback_ends_every_file_block_with_a_blank_line() { std::fs::write(&other, "beta\n").expect("second file"); let (doc, mut store) = app.load_review_file(&other).expect("second store"); store.add(&doc, 0..4, "beta".into(), Kind::Comment, "note B".into()).expect("B"); - app.refresh_review_counts().expect("counts"); + app.refresh_review_counts(); let single = app.feedback(); assert!(single.ends_with("> note A\n\n") && !single.ends_with("\n\n\n"), "{single:?}"); @@ -215,3 +220,74 @@ fn reply_reviews_keep_sending_the_whole_transient_review() { assert!(!draw(&mut app, 80, 24).contains("Finish review")); std::fs::remove_dir_all(root).expect("cleanup"); } + +/// Make the document unreadable. `chmod 000` does it on Unix unless the tests run as root +/// (a container), in which case a directory in its place fails every read the same way. +fn make_unreadable(path: &Path) { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o000)).expect("chmod"); + if std::fs::read(path).is_err() { + return; + } + } + std::fs::remove_file(path).expect("replace the document"); + std::fs::create_dir(path).expect("directory in its place"); +} + +/// One annotated file that cannot be read is left out of the counts with a status note. +/// It never stops the folder from opening, expanding a directory, or reloading. +#[test] +fn a_folder_opens_expands_and_reloads_when_one_annotated_file_is_unreadable() { + let (root, mut app, delivery) = folder_app("unreadable"); + let docs = root.join("docs"); + app.add_quote_annotation("one", Kind::Comment, "readable".into()).expect("A"); + let blocked = docs.join("b.md"); + std::fs::write(&blocked, "beta\n").expect("b"); + let (doc, mut store) = app.load_review_file(&blocked).expect("b store"); + store.add(&doc, 0..4, "beta".into(), Kind::Comment, "cannot be read".into()).expect("B"); + std::fs::create_dir_all(docs.join("deep")).expect("subdir"); + std::fs::write(docs.join("deep/c.md"), "gamma\n").expect("c"); + app.tree = Some(Tree::scan(&docs).expect("tree")); + make_unreadable(&blocked); + assert!(app.load_review_file(&blocked).is_err(), "fixture: the document is unreadable"); + + app.refresh_review_counts(); + assert_eq!(app.unreadable_files, std::slice::from_ref(&blocked)); + assert_eq!(app.status.as_deref(), Some("skipped 1 unreadable file(s): b.md")); + assert_eq!(app.send_count(), 1, "only the readable file counts"); + assert!(!app.folder_counts.contains_key(&blocked)); + + // The note is shown once: an unchanged set does not overwrite a newer status. + app.status = Some("something else".into()); + app.refresh_review_counts(); + assert_eq!(app.status.as_deref(), Some("something else")); + + app.focus = Focus::Tree; + app.tree_cursor = app.tree.as_ref().expect("tree").position(&docs.join("deep")).expect("deep row"); + app.handle_event(&Event::Key(KeyEvent::from(KeyCode::Enter))).expect("expanding a directory"); + assert!(app.tree.as_ref().expect("tree").rows.iter().any(|r| r.path == docs.join("deep/c.md"))); + + app.focus = Focus::Document; + press(&mut app, 'r'); + let status = app.status.as_deref().expect("status"); + assert!(status.starts_with("reloaded ยท 0 orphaned ยท skipped 1 unreadable file(s): b.md"), "{status}"); + assert_eq!(app.send_count(), 1); + + // Opening from scratch resolves the data dir itself, so the record goes there too. + let data_dir = crate::workspace_paths::data_dir(); + let location = Location::for_file(&data_dir, &crate::workspace_paths::project_name(&docs), &blocked); + let doc = Document::parse("beta\n".into()); + let mut store = Store::load(&location, &doc).expect("record in the resolved data dir"); + store.add(&doc, 0..4, "beta".into(), Kind::Comment, "cannot be read".into()).expect("B"); + let opened = App::open_folder(&docs, 100, Box::new(delivery)); + if let Some(record_dir) = location.record.parent() { + std::fs::remove_dir_all(record_dir).expect("cleanup record"); + } + let opened = opened.expect("the folder opens although one annotated file is unreadable"); + assert_eq!(opened.unreadable_files, std::slice::from_ref(&blocked)); + assert_eq!(opened.status.as_deref(), Some("skipped 1 unreadable file(s): b.md")); + assert!(!opened.folder_counts.contains_key(&blocked)); + std::fs::remove_dir_all(root).expect("cleanup"); +} diff --git a/crates/plannotator-tui/src/app/mod.rs b/crates/plannotator-tui/src/app/mod.rs index 4d9e080..282c67e 100644 --- a/crates/plannotator-tui/src/app/mod.rs +++ b/crates/plannotator-tui/src/app/mod.rs @@ -135,6 +135,8 @@ pub(crate) struct App { delivery: Box, send_state: SendState, folder_counts: HashMap, + /// Annotated files the folder counts could not read, in `review_files` order. + unreadable_files: Vec, undo_archive: Vec, archive_items: Vec, archive_cursor: usize, @@ -210,6 +212,7 @@ impl App { delivery, send_state, folder_counts: HashMap::new(), + unreadable_files: Vec::new(), undo_archive: Vec::new(), archive_items: Vec::new(), archive_cursor: 0, @@ -268,7 +271,7 @@ impl App { app.refresh_counts(&mut tree); app.tree_cursor = first.as_deref().and_then(|p| tree.position(p)).unwrap_or(0); app.tree = Some(tree); - app.refresh_review_counts()?; + app.refresh_review_counts(); app.derive_send_state(); Ok(app) } @@ -309,7 +312,7 @@ impl App { self.refresh_counts(&mut tree); self.tree = Some(tree); result?; - self.refresh_review_counts()?; + self.refresh_review_counts(); self.derive_send_state(); } return Ok(()); @@ -468,12 +471,16 @@ impl App { self.open.layout = DocLayout::build(&self.open.doc, self.open.layout.width); self.open.store = Store::load(&Location::for_file(&self.data_dir, &self.project, &path), &self.open.doc)?; - self.refresh_review_counts()?; + self.refresh_review_counts(); self.derive_send_state(); self.sync_tree_counts(); self.clear_selection(); self.selected = self.selected.min(self.open.doc.blocks.len().saturating_sub(1)); - self.status = Some(format!("reloaded ยท {} orphaned", self.open.store.orphans())); + let mut status = format!("reloaded ยท {} orphaned", self.open.store.orphans()); + if let Some(note) = self.unreadable_note() { + status = format!("{status} ยท {note}"); + } + self.status = Some(status); Ok(()) } diff --git a/crates/plannotator-tui/src/app/review/tests.rs b/crates/plannotator-tui/src/app/review/tests.rs index 6770adb..4a0a217 100644 --- a/crates/plannotator-tui/src/app/review/tests.rs +++ b/crates/plannotator-tui/src/app/review/tests.rs @@ -142,11 +142,11 @@ fn folder_finish_reports_partial_failure_and_undo_covers_only_committed_archives std::fs::write(&other, "beta\n\nnew\n").expect("B file"); let (doc, mut store) = app.load_review_file(&other).expect("B store"); store.add(&doc, 0..4, "beta".into(), Kind::Comment, "B".into()).expect("B"); - app.refresh_review_counts().expect("refresh"); + app.refresh_review_counts(); press(&mut app, 'E'); let (doc, mut store) = app.load_review_file(&other).expect("sent B store"); store.add(&doc, 6..9, "new".into(), Kind::Comment, "C pending".into()).expect("C"); - app.refresh_review_counts().expect("refresh"); + app.refresh_review_counts(); let location = Location::for_file(&app.data_dir, &app.project, &other); let blocked_tmp = location.record.with_extension("json.tmp"); std::fs::create_dir(&blocked_tmp).expect("block B archive"); @@ -207,7 +207,7 @@ fn a_deleted_files_archive_is_still_visible_and_restores_when_its_source_returns press(&mut app, 'E'); press(&mut app, 'F'); std::fs::remove_file(&path).expect("source removed"); - app.refresh_review_counts().expect("refresh"); + app.refresh_review_counts(); assert_eq!(app.review_counts().archived, 1); press(&mut app, 'H'); assert_eq!(app.archive_items.len(), 1); diff --git a/crates/plannotator-tui/src/app/review_test_support.rs b/crates/plannotator-tui/src/app/review_test_support.rs index 7de2a6b..258f94a 100644 --- a/crates/plannotator-tui/src/app/review_test_support.rs +++ b/crates/plannotator-tui/src/app/review_test_support.rs @@ -76,7 +76,7 @@ pub(super) fn file_app(tag: &str) -> (PathBuf, App, RecordingDelivery) { pub(super) fn folder_app(tag: &str) -> (PathBuf, App, RecordingDelivery) { let (root, mut app, delivery) = file_app(tag); app.tree = Some(Tree::scan(&root.join("docs")).expect("tree")); - app.refresh_review_counts().expect("counts"); + app.refresh_review_counts(); (root, app, delivery) } @@ -112,6 +112,6 @@ pub(super) fn draw(app: &mut App, width: u16, height: u16) -> String { pub(super) fn reopen(app: &mut App) { let Provenance::File { path } = &app.open.source.provenance else { return }; app.open = Open::new(read_file(path).expect("read"), 100, &app.data_dir, &app.project).expect("reopen"); - app.refresh_review_counts().expect("counts"); + app.refresh_review_counts(); app.derive_send_state(); } diff --git a/crates/plannotator-tui/src/store/review.rs b/crates/plannotator-tui/src/store/review.rs index e3e28cc..d5aac3d 100644 --- a/crates/plannotator-tui/src/store/review.rs +++ b/crates/plannotator-tui/src/store/review.rs @@ -58,12 +58,17 @@ impl Store { } /// Coverage belongs to each annotation's last successful send, not the last batch. - /// Unknown timestamps stay pending rather than silently hiding feedback. + /// A delivery whose time cannot be read covers nothing, so the annotation stays + /// pending rather than silently hiding feedback. An annotation whose own `updated_at` + /// cannot be read is the other way round: once it has been in any delivery it counts as + /// sent, because "pending forever" would resend it on every send and never let it be + /// archived. The next edit rewrites the timestamp and makes it pending again. pub(crate) fn is_pending(&self, annotation: &Annotation) -> bool { - let covered = self.last_delivery(&annotation.id).and_then(|d| parse_time(&d.at)); - match (parse_time(&annotation.updated_at), covered) { + let Some(delivery) = self.last_delivery(&annotation.id) else { return true }; + match (parse_time(&annotation.updated_at), parse_time(&delivery.at)) { (Some(updated), Some(sent)) => updated > sent, - _ => true, + (None, _) => false, + (Some(_), None) => true, } } diff --git a/crates/plannotator-tui/src/store/review/tests.rs b/crates/plannotator-tui/src/store/review/tests.rs index 40d642d..3a8e00e 100644 --- a/crates/plannotator-tui/src/store/review/tests.rs +++ b/crates/plannotator-tui/src/store/review/tests.rs @@ -122,7 +122,10 @@ fn rfc3339_offsets_and_precision_are_compared_as_instants() { ("2026-09-09T05:00:00.001Z", "2026-09-09T05:00:00Z", true), ("2026-09-09T13:00:00+08:00", "2026-09-09T05:00:00Z", false), ("2026-09-09T00:00:00-05:00", "2026-09-09T05:00:00Z", false), - ("unknown", "2026-09-09T05:00:00Z", true), + // An unreadable `updated_at` counts as sent once any delivery covered the id: + // "pending forever" would resend it on every send and never archive it. + ("unknown", "2026-09-09T05:00:00Z", false), + ("unknown", "unknown", false), ("2026-09-09T05:00:00Z", "unknown", true), ] { store.annotations[0].updated_at = updated.into(); @@ -130,6 +133,28 @@ fn rfc3339_offsets_and_precision_are_compared_as_instants() { vec![Delivered { at: sent.into(), target: "agent".into(), annotation_ids: vec![id.clone()] }]; assert_eq!(store.is_pending(&store.annotations[0]), pending, "{updated} vs {sent}"); } + // Never delivered stays pending whatever the timestamp says. + store.deliveries.clear(); + for updated in ["2026-09-09T05:00:00Z", "unknown"] { + store.annotations[0].updated_at = updated.into(); + assert!(store.is_pending(&store.annotations[0]), "{updated} was never sent"); + } +} + +#[test] +fn an_unparsable_timestamp_can_be_finished_and_is_pending_again_after_an_edit() { + let (root, location, doc, mut store) = fixture("unparsable"); + let id = add(&mut store, &doc, "one", "note"); + store.record_delivery("agent", std::slice::from_ref(&id)).expect("send"); + store.annotations[0].updated_at = "not a timestamp".into(); + assert!(!store.is_pending(&store.annotations[0])); + assert!(store.all_delivered()); + assert_eq!(store.archive_sent().expect("finish").as_slice(), std::slice::from_ref(&id)); + assert_eq!(store.restore_archived(&doc, std::slice::from_ref(&id)).expect("restore"), 1); + store.edit_body(&id, "edited".into()).expect("edit"); + assert!(store.is_pending(&store.annotations[0]), "the edit wrote a readable timestamp"); + assert!(Store::load(&location, &doc).expect("reopen").is_pending(&store.annotations[0])); + std::fs::remove_dir_all(root).expect("cleanup"); } #[test] From 0a5e0b475bb1a9ece0bacc5916bf10707f6a1167 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Wed, 9 Sep 2026 12:20:24 -0700 Subject: [PATCH 09/10] feat(tui): put the review actions behind a review menu in the header File and folder reviews showed four header buttons. The header now carries only the send button and a `Review (m)` button. `m` or a click opens a small menu under the header's right edge, drawn like the message picker, with one row per action and a live count: resend all, finish review, undo finish, archive. Rows with nothing to act on are dimmed and skipped; enter, a click, or the row's own key runs a row and closes the menu. The direct keys R, F, U and H keep working without the menu, and the footer undo button is unchanged. Reply reviews keep their header. `--snapshot` accepts a trailing `menu` argument so the open menu can be rendered headlessly. --- crates/plannotator-tui/src/app/draw.rs | 1 + .../plannotator-tui/src/app/feedback/tests.rs | 9 +- crates/plannotator-tui/src/app/header.rs | 31 +-- crates/plannotator-tui/src/app/input.rs | 40 +-- crates/plannotator-tui/src/app/menu.rs | 204 ++++++++++++++ crates/plannotator-tui/src/app/menu/tests.rs | 248 ++++++++++++++++++ crates/plannotator-tui/src/app/mod.rs | 15 +- .../plannotator-tui/src/app/review/tests.rs | 36 --- crates/plannotator-tui/src/cli.rs | 21 +- 9 files changed, 518 insertions(+), 87 deletions(-) create mode 100644 crates/plannotator-tui/src/app/menu.rs create mode 100644 crates/plannotator-tui/src/app/menu/tests.rs diff --git a/crates/plannotator-tui/src/app/draw.rs b/crates/plannotator-tui/src/app/draw.rs index 79921a1..a56778c 100644 --- a/crates/plannotator-tui/src/app/draw.rs +++ b/crates/plannotator-tui/src/app/draw.rs @@ -95,6 +95,7 @@ impl App { Mode::Browse if self.pending.is_some() => self.draw_toolbar(frame), Mode::Pick => self.draw_pick(frame), Mode::Archive => self.draw_archive(frame), + Mode::ReviewMenu => self.draw_review_menu(frame), Mode::Browse | Mode::ConfirmQuit => {} } } diff --git a/crates/plannotator-tui/src/app/feedback/tests.rs b/crates/plannotator-tui/src/app/feedback/tests.rs index 8a79864..d69924f 100644 --- a/crates/plannotator-tui/src/app/feedback/tests.rs +++ b/crates/plannotator-tui/src/app/feedback/tests.rs @@ -28,10 +28,11 @@ fn incremental_send_and_explicit_resend_use_the_same_set_for_body_history_and_id assert_eq!(app.send_count(), 1, "edited B remains pending after reopening"); press(&mut app, 'E'); assert_eq!(app.send_count(), 0); + press(&mut app, 'm'); let screen = draw(&mut app, 80, 24); - assert!(screen.contains("Resend all (3 sent)"), "{screen}"); - let resend = app.geometry.resend_button.expect("resend button"); - click(&mut app, resend); + assert!(screen.contains("R Resend all \u{b7} 3 sent"), "{screen}"); + app.handle_event(&Event::Key(KeyEvent::from(KeyCode::Enter))).expect("resend from the menu"); + assert_eq!(app.mode, Mode::Browse); let calls = delivery.calls.borrow(); assert_eq!(calls.len(), 4); @@ -217,7 +218,7 @@ fn reply_reviews_keep_sending_the_whole_transient_review() { assert_eq!(app.open.store.len(), 2); assert!(app.open.store.archived().is_empty()); assert!(app.open.store.is_transient()); - assert!(!draw(&mut app, 80, 24).contains("Finish review")); + assert!(!draw(&mut app, 80, 24).contains("Review \u{25be}"), "a reply review has no Review menu"); std::fs::remove_dir_all(root).expect("cleanup"); } diff --git a/crates/plannotator-tui/src/app/header.rs b/crates/plannotator-tui/src/app/header.rs index 8206bad..b791029 100644 --- a/crates/plannotator-tui/src/app/header.rs +++ b/crates/plannotator-tui/src/app/header.rs @@ -1,5 +1,5 @@ -//! Visible review actions. Buttons wrap onto another row in a narrow pane instead of -//! disappearing or hiding a single action behind a menu. +//! The header: the send button and, for file and folder reviews, the Review menu button. +//! The two wrap onto another row only when a pane is too narrow for both. use ratatui::Frame; use ratatui::layout::Rect; @@ -15,27 +15,24 @@ const IDLE_BG: Color = Color::Indexed(238); const SENT_BG: Color = Color::Indexed(22); const BLOCKED_BG: Color = Color::Indexed(58); +pub(super) const REVIEW_LABEL: &str = "Review \u{25be} (m)"; + #[derive(Debug, Clone, Copy)] enum Button { Send, - Resend, - Finish, - Archive, + Review, } impl App { + /// Buttons with their rects relative to the header, right to left: send on the edge, + /// the Review menu to its left. fn header_buttons(&self, width: u16) -> Vec<(Button, String, Rect)> { if width == 0 { return Vec::new(); } let mut labels = vec![(Button::Send, self.send_label())]; if self.is_file_review() { - let counts = self.review_counts(); - labels.extend([ - (Button::Resend, format!("Resend all ({} sent) (R)", counts.sent)), - (Button::Finish, "Finish review (F)".into()), - (Button::Archive, format!("Archive {} (H)", counts.archived)), - ]); + labels.push((Button::Review, REVIEW_LABEL.to_owned())); } let mut right = width; let mut y = 0; @@ -71,16 +68,8 @@ impl App { self.geometry.send_button = Some(rect); self.button_style() } - Button::Resend => { - self.geometry.resend_button = Some(rect); - Style::new().fg(Color::Cyan).bg(IDLE_BG) - } - Button::Finish => { - self.geometry.finish_button = Some(rect); - Style::new().fg(Color::Cyan).bg(IDLE_BG) - } - Button::Archive => { - self.geometry.archive_button = Some(rect); + Button::Review => { + self.geometry.review_button = Some(rect); Style::new().fg(Color::Cyan).bg(IDLE_BG) } }; diff --git a/crates/plannotator-tui/src/app/input.rs b/crates/plannotator-tui/src/app/input.rs index f09b677..869b228 100644 --- a/crates/plannotator-tui/src/app/input.rs +++ b/crates/plannotator-tui/src/app/input.rs @@ -7,6 +7,7 @@ use ratatui::crossterm::event::{ }; use super::compose::ComposeAction; +use super::menu::ReviewAction; use super::selection::Selection; use super::send::SendState; use super::{App, Focus, GUTTER, Mode, Pending, TOOLBAR}; @@ -23,6 +24,7 @@ impl App { self.archive_key(*key); Ok(()) } + Mode::ReviewMenu => self.menu_key(*key), Mode::Compose | Mode::Edit(_) => self.text_key(*key), }, // A paste lands in the comment box verbatim, newlines included; anywhere else @@ -33,6 +35,7 @@ impl App { } Event::Mouse(mouse) if self.mode == Mode::Browse => self.mouse(*mouse), Event::Mouse(mouse) if self.mode == Mode::Pick => self.pick_mouse(*mouse), + Event::Mouse(mouse) if self.mode == Mode::ReviewMenu => self.menu_mouse(*mouse), Event::Mouse(mouse) if self.mode == Mode::Archive => { self.archive_mouse(*mouse); Ok(()) @@ -53,18 +56,21 @@ impl App { return Ok(()); } (KeyCode::Char('E'), _) => return self.send_feedback(), - (KeyCode::Char('R'), _) if self.is_file_review() => return self.resend_all(), - (KeyCode::Char('F'), _) if self.is_file_review() => { - self.finish_review(); + (KeyCode::Char('m'), _) if self.is_file_review() => { + self.open_review_menu(); return Ok(()); } - (KeyCode::Char('H'), _) if self.is_file_review() => { - self.open_archive(); - return Ok(()); + (KeyCode::Char('R'), _) if self.is_file_review() => { + return self.run_review_action(ReviewAction::ResendAll); + } + (KeyCode::Char('F'), _) if self.is_file_review() => { + return self.run_review_action(ReviewAction::Finish); } (KeyCode::Char('U'), _) if self.is_file_review() => { - self.undo_finish_review(); - return Ok(()); + return self.run_review_action(ReviewAction::Undo); + } + (KeyCode::Char('H'), _) if self.is_file_review() => { + return self.run_review_action(ReviewAction::Archive); } (KeyCode::Char('t'), _) => { self.toggle_tree(self.geometry.doc.width + self.geometry.tree.width + GUTTER); @@ -295,7 +301,12 @@ impl App { self.status = Some("annotation updated".into()); } } - Mode::Compose | Mode::Browse | Mode::ConfirmQuit | Mode::Pick | Mode::Archive => { + Mode::Compose + | Mode::Browse + | Mode::ConfirmQuit + | Mode::Pick + | Mode::Archive + | Mode::ReviewMenu => { if !body.is_empty() && let Some(pending) = self.pending.take() { @@ -324,15 +335,8 @@ impl App { let hit = |rect: Option| { rect.is_some_and(|r| mouse.row == r.y && mouse.column >= r.x && mouse.column < r.right()) }; - if hit(self.geometry.resend_button) { - return self.resend_all(); - } - if hit(self.geometry.finish_button) { - self.finish_review(); - return Ok(()); - } - if hit(self.geometry.archive_button) { - self.open_archive(); + if hit(self.geometry.review_button) { + self.open_review_menu(); return Ok(()); } if hit(self.geometry.undo_button) { diff --git a/crates/plannotator-tui/src/app/menu.rs b/crates/plannotator-tui/src/app/menu.rs new file mode 100644 index 0000000..3ad0fec --- /dev/null +++ b/crates/plannotator-tui/src/app/menu.rs @@ -0,0 +1,204 @@ +//! The Review menu: the file and folder review actions behind one header button. `m` or +//! a click opens it under the header's right edge; it is drawn like the message picker. + +use anyhow::Result; +use ratatui::Frame; +use ratatui::crossterm::event::{KeyCode, KeyEvent, KeyModifiers, MouseButton, MouseEvent, MouseEventKind}; +use ratatui::layout::Rect; +use ratatui::style::{Color, Style}; +use ratatui::text::{Line, Span}; +use ratatui::widgets::{Block, BorderType, Borders, Clear, Paragraph}; +use unicode_width::UnicodeWidthStr as _; + +use super::{App, Mode}; + +#[cfg(test)] +mod tests; + +/// Menu rows, in display order. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(super) enum ReviewAction { + ResendAll, + Finish, + Undo, + Archive, +} + +const REVIEW_ACTIONS: [ReviewAction; 4] = + [ReviewAction::ResendAll, ReviewAction::Finish, ReviewAction::Undo, ReviewAction::Archive]; + +const MENU_HELP: &str = " \u{2191}\u{2193} move \u{b7} enter run \u{b7} esc close "; + +impl ReviewAction { + /// The key that runs the action, from the menu and directly in the review. + fn key(self) -> char { + match self { + Self::ResendAll => 'R', + Self::Finish => 'F', + Self::Undo => 'U', + Self::Archive => 'H', + } + } + + fn from_key(key: char) -> Option { + REVIEW_ACTIONS.into_iter().find(|action| action.key() == key) + } +} + +impl App { + fn action_label(&self, action: ReviewAction) -> String { + let counts = self.review_counts(); + match action { + ReviewAction::ResendAll => format!("Resend all \u{b7} {} sent", counts.sent), + ReviewAction::Finish => format!("Finish review \u{b7} archive {} sent", counts.sent), + ReviewAction::Undo => "Undo finish".to_owned(), + ReviewAction::Archive => format!("Archive \u{b7} {} notes", counts.archived), + } + } + + /// Whether the action has anything to act on. Other rows are dimmed and skipped. + fn action_applies(&self, action: ReviewAction) -> bool { + let counts = self.review_counts(); + match action { + ReviewAction::ResendAll | ReviewAction::Finish => counts.sent > 0, + ReviewAction::Undo => !self.undo_archive.is_empty(), + ReviewAction::Archive => counts.archived > 0, + } + } + + /// Open the menu on the first row that applies. Reply reviews have no menu. + pub(crate) fn open_review_menu(&mut self) { + if !self.is_file_review() { + return; + } + self.menu_cursor = REVIEW_ACTIONS.iter().position(|&action| self.action_applies(action)).unwrap_or(0); + self.mode = Mode::ReviewMenu; + } + + /// Leave the menu, then act exactly as the direct key does. + pub(super) fn run_review_action(&mut self, action: ReviewAction) -> Result<()> { + self.mode = Mode::Browse; + match action { + ReviewAction::ResendAll => self.resend_all(), + ReviewAction::Finish => { + self.finish_review(); + Ok(()) + } + ReviewAction::Undo => { + self.undo_finish_review(); + Ok(()) + } + ReviewAction::Archive => { + self.open_archive(); + Ok(()) + } + } + } + + /// Move to the next row that applies in the given direction; stay when there is none. + fn move_menu_cursor(&mut self, down: bool) { + let applies = |(index, action): (usize, &ReviewAction)| self.action_applies(*action).then_some(index); + let rows = REVIEW_ACTIONS.iter().enumerate(); + let next = if down { + rows.skip(self.menu_cursor + 1).find_map(applies) + } else { + rows.take(self.menu_cursor).filter_map(applies).next_back() + }; + if let Some(index) = next { + self.menu_cursor = index; + } + } + + pub(super) fn menu_key(&mut self, key: KeyEvent) -> Result<()> { + match key.code { + KeyCode::Char('j') | KeyCode::Down => self.move_menu_cursor(true), + KeyCode::Char('k') | KeyCode::Up => self.move_menu_cursor(false), + KeyCode::Enter => { + if let Some(&action) = REVIEW_ACTIONS.get(self.menu_cursor) + && self.action_applies(action) + { + return self.run_review_action(action); + } + } + KeyCode::Esc | KeyCode::Char('m') => self.mode = Mode::Browse, + KeyCode::Char('c') if key.modifiers == KeyModifiers::CONTROL => self.mode = Mode::Browse, + KeyCode::Char(ch) => { + if let Some(action) = ReviewAction::from_key(ch) { + return self.run_review_action(action); + } + } + _ => {} + } + Ok(()) + } + + /// A row that applies runs on click; a click outside the menu closes it. + pub(super) fn menu_mouse(&mut self, mouse: MouseEvent) -> Result<()> { + if mouse.kind != MouseEventKind::Down(MouseButton::Left) { + return Ok(()); + } + let inside = |rect: Rect| { + mouse.column >= rect.x + && mouse.column < rect.right() + && mouse.row >= rect.y + && mouse.row < rect.bottom() + }; + let hit = self.geometry.menu_rows.iter().find(|(rect, _)| inside(*rect)).map(|(_, index)| *index); + match hit.and_then(|index| REVIEW_ACTIONS.get(index).copied()) { + Some(action) if self.action_applies(action) => self.run_review_action(action), + Some(_) => Ok(()), + None => { + if !self.geometry.menu.is_some_and(inside) { + self.mode = Mode::Browse; + } + Ok(()) + } + } + } + + pub(super) fn draw_review_menu(&mut self, frame: &mut Frame) { + let area = frame.area(); + let rows: Vec<(ReviewAction, String)> = REVIEW_ACTIONS + .iter() + .map(|&action| (action, format!(" {} {} ", action.key(), self.action_label(action)))) + .collect(); + let content = rows.iter().map(|(_, text)| text.width()).max().unwrap_or(0).max(MENU_HELP.width()); + let width = (content as u16 + 2).min(area.width); + let height = (REVIEW_ACTIONS.len() as u16 + 2).min(area.height); + // Under the Review button, flush with its right edge, like a dropdown. + let button = self.geometry.review_button; + let x = button.map_or(area.right(), Rect::right).saturating_sub(width).max(area.x); + let y = button.map_or(area.y, Rect::bottom).min(area.bottom().saturating_sub(height)); + let rect = Rect { x, y, width, height }; + frame.render_widget(Clear, rect); + let boxed = Block::default() + .borders(Borders::ALL) + .border_type(BorderType::Rounded) + .border_style(Style::new().fg(Color::Cyan)) + .title(Span::styled(" review ", Style::new().dim())) + .title_bottom(Span::styled(MENU_HELP, Style::new().dim())); + let inner = boxed.inner(rect); + frame.render_widget(boxed, rect); + let mut menu_rows = Vec::new(); + let lines: Vec> = rows + .into_iter() + .enumerate() + .take(usize::from(inner.height)) + .map(|(index, (action, text))| { + let row = Rect { x: inner.x, y: inner.y + index as u16, width: inner.width, height: 1 }; + menu_rows.push((row, index)); + let style = if !self.action_applies(action) { + Style::new().dim() + } else if index == self.menu_cursor { + Style::new().reversed() + } else { + Style::new() + }; + Line::from(Span::styled(format!("{text: Vec<(bool, bool)> { + let mut terminal = Terminal::new(TestBackend::new(100, 24)).expect("terminal"); + terminal.draw(|frame| app.draw(frame)).expect("draw"); + let buffer = terminal.backend().buffer(); + app.geometry + .menu_rows + .iter() + .map(|(rect, _)| { + let modifiers = buffer.cell((rect.x, rect.y)).expect("cell").style().add_modifier; + (modifiers.contains(Modifier::DIM), modifiers.contains(Modifier::REVERSED)) + }) + .collect() +} + +#[test] +fn the_header_holds_send_and_review_and_m_opens_and_closes_the_menu() { + let (root, mut app, _) = file_app("menu-open"); + app.add_quote_annotation("one", Kind::Comment, "A".into()).expect("A"); + let screen = draw(&mut app, 100, 24); + let header = screen.lines().next().expect("header"); + assert!( + header.contains("Review \u{25be} (m)") && header.contains("Send 1 new \u{25b8} test agent (E)"), + "{header}" + ); + for gone in ["Resend all", "Finish review", "Archive"] { + assert!(!header.contains(gone), "{gone} is behind the menu now: {header}"); + } + let review = app.geometry.review_button.expect("review button"); + let send = app.geometry.send_button.expect("send button"); + assert!(review.right() < send.x && send.right() == 100, "review sits left of send: {review:?} {send:?}"); + + press(&mut app, 'm'); + assert_eq!(app.mode, Mode::ReviewMenu); + let screen = draw(&mut app, 100, 24); + for row in [ + "R Resend all \u{b7} 0 sent", + "F Finish review \u{b7} archive 0 sent", + "U Undo finish", + "H Archive \u{b7} 0 notes", + ] { + assert!(screen.contains(row), "{row} missing:\n{screen}"); + } + assert!(screen.contains(" review "), "{screen}"); + let menu = app.geometry.menu.expect("menu rect"); + assert_eq!(menu.y, review.bottom(), "the menu hangs under the header"); + assert_eq!(menu.right(), review.right(), "flush with the button's right edge"); + + press(&mut app, 'm'); + assert_eq!(app.mode, Mode::Browse, "m closes the menu"); + press(&mut app, 'm'); + key(&mut app, KeyCode::Esc); + assert_eq!(app.mode, Mode::Browse, "esc closes the menu"); + assert!(!draw(&mut app, 100, 24).contains("Resend all")); + std::fs::remove_dir_all(root).expect("cleanup"); +} + +#[test] +fn movement_skips_dimmed_rows_and_opens_on_the_first_that_applies() { + let (root, mut app, _) = file_app("menu-skip"); + app.add_quote_annotation("one", Kind::Comment, "A".into()).expect("A"); + press(&mut app, 'E'); + press(&mut app, 'm'); + // One sent note: resend and finish apply; nothing to undo and the archive is empty. + assert_eq!(app.menu_cursor, 0); + assert_eq!(row_styles(&mut app), [(false, true), (false, false), (true, false), (true, false)]); + press(&mut app, 'j'); + assert_eq!(app.menu_cursor, 1); + press(&mut app, 'j'); + assert_eq!(app.menu_cursor, 1, "undo and archive are dimmed, so the cursor stays"); + key(&mut app, KeyCode::Up); + assert_eq!(app.menu_cursor, 0); + key(&mut app, KeyCode::Up); + assert_eq!(app.menu_cursor, 0); + + press(&mut app, 'F'); + assert_eq!(app.mode, Mode::Browse); + press(&mut app, 'm'); + // Now nothing is sent: resend and finish are dimmed; undo and archive apply. + assert_eq!(app.menu_cursor, 2, "opens on the first row that applies"); + assert_eq!(row_styles(&mut app), [(true, false), (true, false), (false, true), (false, false)]); + press(&mut app, 'k'); + assert_eq!(app.menu_cursor, 2); + key(&mut app, KeyCode::Down); + assert_eq!(app.menu_cursor, 3); + key(&mut app, KeyCode::Down); + assert_eq!(app.menu_cursor, 3); + std::fs::remove_dir_all(root).expect("cleanup"); +} + +#[test] +fn enter_runs_the_highlighted_row_and_closes_the_menu() { + let (root, mut app, delivery) = file_app("menu-enter"); + app.add_quote_annotation("one", Kind::Comment, "A".into()).expect("A"); + press(&mut app, 'E'); + press(&mut app, 'm'); + key(&mut app, KeyCode::Enter); + assert_eq!(app.mode, Mode::Browse); + assert_eq!(delivery.calls.borrow().len(), 2, "enter on the first row resent"); + assert!(app.status.as_deref().expect("status").starts_with("sent 1 annotation(s)"), "{:?}", app.status); + + press(&mut app, 'm'); + press(&mut app, 'j'); + key(&mut app, KeyCode::Enter); + assert_eq!(app.mode, Mode::Browse); + assert_eq!(app.status.as_deref(), Some("archived 1 annotation(s)")); + assert_eq!(app.open.store.archived().len(), 1); + + press(&mut app, 'm'); + assert_eq!(app.menu_cursor, 2); + key(&mut app, KeyCode::Enter); + assert_eq!(app.mode, Mode::Browse); + assert_eq!(app.status.as_deref(), Some("restored 1 annotation(s)")); + assert_eq!(app.open.store.len(), 1); + std::fs::remove_dir_all(root).expect("cleanup"); +} + +#[test] +fn a_rows_own_key_runs_it_from_the_menu() { + let (root, mut app, _) = file_app("menu-keys"); + app.add_quote_annotation("one", Kind::Comment, "A".into()).expect("A"); + press(&mut app, 'E'); + press(&mut app, 'm'); + press(&mut app, 'F'); + assert_eq!(app.mode, Mode::Browse); + assert_eq!(app.status.as_deref(), Some("archived 1 annotation(s)")); + + press(&mut app, 'm'); + press(&mut app, 'H'); + assert_eq!(app.mode, Mode::Archive); + assert_eq!(app.archive_items.len(), 1); + key(&mut app, KeyCode::Esc); + + press(&mut app, 'm'); + press(&mut app, 'R'); + assert_eq!(app.mode, Mode::Browse); + assert_eq!(app.status.as_deref(), Some("nothing to send"), "the direct key's own message"); + + press(&mut app, 'm'); + press(&mut app, 'U'); + assert_eq!(app.mode, Mode::Browse); + assert_eq!(app.status.as_deref(), Some("restored 1 annotation(s)")); + std::fs::remove_dir_all(root).expect("cleanup"); +} + +#[test] +fn the_menu_is_unavailable_while_composing_and_in_reply_reviews() { + let (root, mut app, _) = file_app("menu-gate"); + press(&mut app, 'c'); + assert_eq!(app.mode, Mode::Compose); + press(&mut app, 'm'); + assert_eq!(app.mode, Mode::Compose, "m is text while composing"); + key(&mut app, KeyCode::Enter); + assert_eq!(app.open.store.placed()[0].annotation.body, "m"); + + let source = DocumentSource::new( + "one\n\ntwo\n".into(), + "agent reply", + true, + Provenance::AgentMessage { + host: "claude".into(), + session: None, + message_id: Some("message-1".into()), + }, + ); + app.open = Open::new(source, 80, &app.data_dir, &app.project).expect("reply"); + let screen = draw(&mut app, 100, 24); + assert!(!screen.contains("Review \u{25be}"), "{screen}"); + assert!(app.geometry.review_button.is_none()); + press(&mut app, 'm'); + assert_eq!(app.mode, Mode::Browse, "a reply review has no menu"); + std::fs::remove_dir_all(root).expect("cleanup"); +} + +#[test] +fn a_narrow_pane_keeps_both_header_buttons_on_one_line() { + let (root, mut app, _) = file_app("menu-narrow"); + app.add_quote_annotation("one", Kind::Comment, "A".into()).expect("A"); + let screen = draw(&mut app, 60, 24); + let mut lines = screen.lines(); + let header = lines.next().expect("header"); + let second = lines.next().expect("second row"); + assert_eq!(app.header_height(60), 1); + assert!(header.contains("Review \u{25be} (m)") && header.contains("(E)"), "{header}"); + assert!(!second.contains("Review") && !second.contains("(E)"), "nothing wrapped: {second}"); + let review = app.geometry.review_button.expect("review"); + let send = app.geometry.send_button.expect("send"); + assert_eq!((review.y, send.y), (0, 0)); + assert!(review.right() < send.x && send.right() == 60); + std::fs::remove_dir_all(root).expect("cleanup"); +} + +#[test] +fn clicking_the_review_button_opens_the_menu_and_clicking_a_row_runs_it() { + let (root, mut app, _) = file_app("menu-click"); + app.add_quote_annotation("one", Kind::Comment, "A".into()).expect("A"); + press(&mut app, 'E'); + draw(&mut app, 80, 24); + let review = app.geometry.review_button.expect("review"); + click(&mut app, review); + assert_eq!(app.mode, Mode::ReviewMenu); + draw(&mut app, 80, 24); + let finish = app.geometry.menu_rows[1].0; + click(&mut app, finish); + assert_eq!(app.mode, Mode::Browse); + assert_eq!(app.open.store.archived().len(), 1); + + press(&mut app, 'm'); + draw(&mut app, 80, 24); + let resend = app.geometry.menu_rows[0].0; + click(&mut app, resend); + assert_eq!(app.mode, Mode::ReviewMenu, "a dimmed row ignores the click"); + let archive = app.geometry.menu_rows[3].0; + click(&mut app, archive); + assert_eq!(app.mode, Mode::Archive); + key(&mut app, KeyCode::Esc); + + press(&mut app, 'm'); + draw(&mut app, 80, 24); + let outside = Rect { x: 0, y: 12, width: 1, height: 1 }; + app.handle_event(&Event::Mouse(MouseEvent { + kind: MouseEventKind::Down(MouseButton::Left), + column: outside.x, + row: outside.y, + modifiers: KeyModifiers::NONE, + })) + .expect("click outside"); + assert_eq!(app.mode, Mode::Browse, "a click outside the menu closes it"); + std::fs::remove_dir_all(root).expect("cleanup"); +} diff --git a/crates/plannotator-tui/src/app/mod.rs b/crates/plannotator-tui/src/app/mod.rs index 282c67e..c1c6925 100644 --- a/crates/plannotator-tui/src/app/mod.rs +++ b/crates/plannotator-tui/src/app/mod.rs @@ -7,7 +7,7 @@ mod draw; mod feedback; mod header; mod input; - +mod menu; mod pick; mod review; #[cfg(test)] @@ -57,6 +57,8 @@ enum Mode { Pick, /// Restoring annotations from finished file reviews. Archive, + /// The header's Review menu is open over a file or folder review. + ReviewMenu, } /// Which pane keyboard input goes to. @@ -78,10 +80,12 @@ struct Geometry { bubbles: Vec<(Rect, String)>, /// The header's Send button; `None` when the header was too narrow for it. send_button: Option, - resend_button: Option, - finish_button: Option, - archive_button: Option, + /// The header's Review button; only file and folder reviews draw it. + review_button: Option, undo_button: Option, + /// The Review menu drawn last frame and its rows, with their action index. + menu: Option, + menu_rows: Vec<(Rect, usize)>, archive_rows: Vec<(Rect, usize)>, /// Picker rows drawn last frame, with their candidate index. pick_rows: Vec<(Rect, usize)>, @@ -140,6 +144,8 @@ pub(crate) struct App { undo_archive: Vec, archive_items: Vec, archive_cursor: usize, + /// The highlighted row of the Review menu. + menu_cursor: usize, focus: Focus, scroll: usize, selected: usize, @@ -216,6 +222,7 @@ impl App { undo_archive: Vec::new(), archive_items: Vec::new(), archive_cursor: 0, + menu_cursor: 0, focus: Focus::Document, scroll: 0, selected: 0, diff --git a/crates/plannotator-tui/src/app/review/tests.rs b/crates/plannotator-tui/src/app/review/tests.rs index 4a0a217..2c83824 100644 --- a/crates/plannotator-tui/src/app/review/tests.rs +++ b/crates/plannotator-tui/src/app/review/tests.rs @@ -71,42 +71,6 @@ fn sent_markers_clear_on_edit_and_review_shortcuts_are_text_while_composing() { std::fs::remove_dir_all(root).expect("cleanup"); } -#[test] -fn every_review_action_stays_visible_and_clickable_in_a_narrow_pane() { - let (root, mut app, _) = file_app("buttons"); - app.add_quote_annotation("one", Kind::Comment, "note".into()).expect("note"); - press(&mut app, 'E'); - for width in [80, 40] { - let screen = draw(&mut app, width, 24); - let buttons = [ - app.geometry.send_button, - app.geometry.resend_button, - app.geometry.finish_button, - app.geometry.archive_button, - ]; - for rect in buttons.into_iter().map(|r| r.expect("visible button")) { - assert!(rect.right() <= width && rect.bottom() < 24, "{screen}"); - } - assert!( - screen.contains("Finish review") && screen.contains("Archive 0") && screen.contains("Resend all"), - "{screen}" - ); - } - let finish = app.geometry.finish_button.expect("finish"); - click(&mut app, finish); - assert_eq!(app.open.store.len(), 0); - draw(&mut app, 40, 24); - let archive = app.geometry.archive_button.expect("archive"); - click(&mut app, archive); - assert_eq!(app.mode, Mode::Archive); - draw(&mut app, 40, 24); - let row = app.geometry.archive_rows[0].0; - click(&mut app, row); - assert_eq!(app.open.store.len(), 1); - assert_eq!(app.send_count(), 0); - std::fs::remove_dir_all(root).expect("cleanup"); -} - #[test] fn the_archive_picker_scrolls_and_restores_the_clicked_annotation() { let (root, mut app, _) = file_app("archive-scroll"); diff --git a/crates/plannotator-tui/src/cli.rs b/crates/plannotator-tui/src/cli.rs index f040389..2d0a3db 100644 --- a/crates/plannotator-tui/src/cli.rs +++ b/crates/plannotator-tui/src/cli.rs @@ -30,7 +30,7 @@ const USAGE: &str = "usage: plannotator-tui --blocks plannotator-tui --annotate [comment|looks_good|delete] plannotator-tui --annotate-block - plannotator-tui --snapshot [cols rows scroll] [select-quote] + plannotator-tui --snapshot [cols rows scroll] [select-quote] [menu] plannotator-tui config plannotator-tui --version plannotator-tui herdr open [file.md | folder] [--placement overlay|split|popup] [--deliver-to ] @@ -117,7 +117,10 @@ pub(crate) fn run(args: &[String]) -> Result<()> { let cols: u16 = arg(2).and_then(|s| s.parse().ok()).unwrap_or(140); let rows: u16 = arg(3).and_then(|s| s.parse().ok()).unwrap_or(40); let scroll: i64 = arg(4).and_then(|s| s.parse().ok()).unwrap_or(0); - snapshot(&path(1)?, cols, rows, scroll, arg(5)) + // A trailing `menu` opens the Review menu; a quote may come before it. + let menu = args.get(5..).is_some_and(|rest| rest.last().is_some_and(|last| last == "menu")); + let select = arg(5).filter(|quote| *quote != "menu"); + snapshot(&path(1)?, cols, rows, scroll, select, menu) } Some("--version" | "-V") => { println!("plannotator-tui {}", env!("CARGO_PKG_VERSION")); @@ -334,8 +337,15 @@ fn bench(path: &PathBuf) -> Result<()> { } /// Draw one frame into an in-memory backend and print it as text, followed by a mark map: -/// `#` comment, `+` looks good, `-` delete, `%` selected. -fn snapshot(path: &PathBuf, cols: u16, rows: u16, scroll: i64, select: Option<&str>) -> Result<()> { +/// `#` comment, `+` looks good, `-` delete, `%` selected or highlighted. +fn snapshot( + path: &PathBuf, + cols: u16, + rows: u16, + scroll: i64, + select: Option<&str>, + menu: bool, +) -> Result<()> { use ratatui::backend::TestBackend; use ratatui::style::{Color, Modifier}; let mut terminal = ratatui::Terminal::new(TestBackend::new(cols, rows))?; @@ -345,6 +355,9 @@ fn snapshot(path: &PathBuf, cols: u16, rows: u16, scroll: i64, select: Option<&s if let Some(quote) = select { app.select_quote_for_snapshot(quote)?; } + if menu { + app.open_review_menu(); + } terminal.draw(|frame| app.draw(frame))?; let buffer = terminal.backend().buffer(); let mut marks = Vec::new(); From ac454bcb918472860ffba6d2ab94b11f0fb116cf Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Wed, 9 Sep 2026 12:20:24 -0700 Subject: [PATCH 10/10] docs: describe the review menu in the key tables --- README.md | 11 +++++++---- crates/plannotator-tui/README.md | 8 +++++--- 2 files changed, 12 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 164d614..639af16 100644 --- a/README.md +++ b/README.md @@ -60,14 +60,17 @@ they reach your own machine even when the app runs on a remote server; Herdr Ann For file and folder reviews, `E` sends only new or edited annotations. Send A and B, then add C: the next send includes just C. Sent notes stay visible with a marker; editing one makes it pending again, including after a restart. `R` **Resend all** includes every active -note, with the button showing how many are already sent. With nothing pending, `E` reports -โ€œnothing new to sendโ€. A failed send keeps the notes pending for retry. +note. With nothing pending, `E` reports โ€œnothing new to sendโ€. A failed send keeps the notes +pending for retry. `F` **Finish review** archives sent, unchanged notes and leaves pending ones in place. `U` undoes the last finish during this session. `H` opens the archive, where Enter or a click restores a note even after reopening the app. Restoring keeps its original id and sent status. The archive is stored with the annotations and works even when feedback -history is turned off. +history is turned off. The header holds the send button and a `Review โ–พ (m)` button whose +menu lists these four actions with live counts (`R` resend all ยท 3 sent, `F` finish review +ยท archive 3 sent, `U` undo finish, `H` archive ยท 2 notes); rows with nothing to act on are +dimmed. The keys also work without opening the menu. | Where | Keys | |---|---| @@ -75,7 +78,7 @@ history is turned off. | document | `j`/`k` block; `c` comment on the block; `x` clear its annotations; `v` select with `hjkl` `w` `b` `0` `$` | | toolbar | `a` looks good ยท `c` comment ยท `d` delete ยท `Esc` | | notes | `j`/`k`; `e` edit; `x` remove; click a bubble | -| file/folder review | `E` send new ยท `R` resend all ยท `F` finish review ยท `U` undo finish ยท `H` archive | +| file/folder review | `E` send new ยท `m` review menu (`R` resend all ยท `F` finish review ยท `U` undo ยท `H` archive) | | tree | `j`/`k`; `Enter` open; `E` sends new notes across all reviewed files, including collapsed folders | ## Inside Herdr diff --git a/crates/plannotator-tui/README.md b/crates/plannotator-tui/README.md index c90f11e..384bbf2 100644 --- a/crates/plannotator-tui/README.md +++ b/crates/plannotator-tui/README.md @@ -16,7 +16,7 @@ cargo build --release | document | drag with the mouse, or `v` then `hjkl` / `w` `b` / `0` `$` to select; `Enter` confirms ยท `j`/`k` or click selects a block ยท `c` comments on the block ยท `x` clears the block's annotations | | selection toolbar | `a` ๐Ÿ‘ looks good ยท `c` ๐Ÿ’ฌ comment (opens a box at the selection) ยท `d` โœ— delete ยท `Esc` clears | | rail | `j`/`k` move ยท `e` / `Enter` edit body ยท `x` remove ยท click a bubble to focus it | -| file/folder review | `E` send new or edited notes ยท `R` resend all active notes ยท `F` archive sent notes ยท `U` undo finish ยท `H` open archive | +| file/folder review | `E` send new ยท `m` review menu (`R` resend all ยท `F` finish review ยท `U` undo ยท `H` archive) | | tree | `j`/`k` move ยท `Enter` open ยท `E` send new feedback across files, including collapsed folders ยท counts show active notes per file | | archive | `j`/`k` or โ†‘/โ†“ select ยท `Enter` or click restore ยท `Esc` close | @@ -25,8 +25,10 @@ Selections and exports are copied to the terminal clipboard (OSC 52). Sent notes remain visible. File reviews send only new or edited notes by default; `R` explicitly includes the sent ones too. `F` archives only sent notes that have not changed since delivery. Archived notes survive restarts, and restoring them keeps their -ids and delivery history. Failed sends keep the feedback pending. These actions also -have clickable buttons, which wrap onto another header row in narrow panes. +ids and delivery history. Failed sends keep the feedback pending. The header shows the +send button and a `Review โ–พ (m)` button; its menu lists the four actions with live counts, +dims the ones with nothing to act on, and runs a row on `Enter`, a click, or the row's own +key. The keys also work directly, without the menu. ## Where things live