fix(review): send and count notes on every reply in a multi-reply review

crates/plannotator-tui/src/app/feedback.rs

@@ -40,6 +40,8 @@ impl ReviewCounts {
 #[derive(Debug)]
 pub(super) struct FeedbackPart {
     pub(super) path: Option<PathBuf>,
+    /// The reply these notes belong to, in a reply review; `None` for files.
+    pub(super) reply: Option<usize>,
     pub(super) store: Store,
     pub(super) ids: Vec<String>,
 }
@@ -54,7 +56,15 @@ pub(super) struct Feedback {
 }
 
 impl Feedback {
-    fn add(&mut self, path: Option<PathBuf>, name: &str, doc: &Document, store: Store, scope: SendScope) {
+    pub(super) fn add(
+        &mut self,
+        path: Option<PathBuf>,
+        reply: Option<usize>,
+        name: &str,
+        doc: &Document,
+        store: Store,
+        scope: SendScope,
+    ) {
         if let Some(path) = &path {
             self.counts.insert(path.clone(), ReviewCounts::for_store(&store));
         }
@@ -93,7 +103,7 @@ impl Feedback {
                 original_text: (!a.anchor.original_text.is_empty()).then(|| a.anchor.original_text.clone()),
             }
         }));
-        self.parts.push(FeedbackPart { path, store, ids });
+        self.parts.push(FeedbackPart { path, reply, store, ids });
     }
 
     fn exported_text(self) -> String {
@@ -234,12 +244,17 @@ impl App {
             _ => None,
         };
         let mut feedback = Feedback::default();
-        feedback.add(path, &self.open.source.name, &self.open.doc, self.open.store.clone(), scope);
+        feedback.add(path, None, &self.open.source.name, &self.open.doc, self.open.store.clone(), scope);
         feedback
     }
 
     pub(super) fn prepare_feedback(&self, scope: SendScope) -> Result<Feedback> {
-        let Some(tree) = &self.tree else { return Ok(self.file_feedback(scope)) };
+        let Some(tree) = &self.tree else {
+            if !self.pick_cache.is_empty() {
+                return Ok(self.reply_feedback(scope));
+            }
+            return Ok(self.file_feedback(scope));
+        };
         let mut feedback = Feedback::default();
         for path in self.review_files() {
             if !path.is_file() {
@@ -249,10 +264,10 @@ impl App {
             }
             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);
+                feedback.add(Some(path), None, &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);
+                feedback.add(Some(path), None, &name, &doc, store, scope);
             }
         }
         // Folder feedback has always ended each file's block with one extra newline, so

crates/plannotator-tui/src/app/mod.rs

@@ -9,6 +9,7 @@ mod header;
 mod input;
 mod menu;
 mod pick;
+mod replies;
 mod review;
 #[cfg(test)]
 mod review_test_support;

crates/plannotator-tui/src/app/replies.rs

@@ -0,0 +1,67 @@
+//! A reply review that has opened more than one reply is still one review. Each reply
+//! keeps its notes in memory (the open one in `open`, the rest in `pick_cache`), and
+//! sending, counting and the quit question cover all of them.
+
+use super::feedback::{Feedback, SendScope};
+use super::{App, Open};
+use crate::store::Store;
+
+impl App {
+    /// Every reply this review has opened, with its candidate index, in picker order.
+    /// A file, folder or single-document review has just the open document.
+    pub(super) fn replies(&self) -> Vec<(usize, &Open)> {
+        let mut replies: Vec<(usize, &Open)> = std::iter::once((self.pick_open, &self.open))
+            .chain(self.pick_cache.iter().map(|(index, open)| (*index, open)))
+            .collect();
+        replies.sort_by_key(|(index, _)| *index);
+        replies
+    }
+
+    pub(super) fn reply_store_mut(&mut self, index: usize) -> Option<&mut Store> {
+        if index == self.pick_open {
+            Some(&mut self.open.store)
+        } else {
+            self.pick_cache.get_mut(&index).map(|open| &mut open.store)
+        }
+    }
+
+    /// Notes from every reply that has any. One such reply is sent exactly as a review
+    /// that never used the picker; two or more are sent one block per reply, each
+    /// headed with which reply it is so the agent can tell them apart. A reply that is
+    /// not open and whose notes were all delivered already is left out, so sending from
+    /// another reply does not repeat it.
+    pub(super) fn reply_feedback(&self, scope: SendScope) -> Feedback {
+        let annotated: Vec<(usize, &Open)> = self
+            .replies()
+            .into_iter()
+            .filter(|(_, open)| !open.store.placed().is_empty())
+            .filter(|(index, open)| *index == self.pick_open || !open.store.all_delivered())
+            .collect();
+        let mut feedback = Feedback::default();
+        let named = annotated.len() > 1;
+        for (index, open) in annotated {
+            let name = if named { self.reply_name(index, open) } else { open.source.name.clone() };
+            feedback.add(None, Some(index), &name, &open.doc, open.store.clone(), scope);
+        }
+        feedback
+    }
+
+    /// `claude · message 2 of 3 ("first line")`, numbered as the picker lists them.
+    fn reply_name(&self, index: usize, open: &Open) -> String {
+        let host = &self.message_host;
+        let total = self.candidates.len();
+        let first = open
+            .doc
+            .source
+            .lines()
+            .map(|line| line.trim().trim_start_matches('#').trim())
+            .find(|line| !line.is_empty())
+            .unwrap_or("");
+        let first: String = if first.chars().count() > 60 {
+            first.chars().take(59).chain(std::iter::once('…')).collect()
+        } else {
+            first.to_owned()
+        };
+        format!("{host} · message {} of {total} (\"{first}\")", index + 1)
+    }
+}

crates/plannotator-tui/src/app/send.rs

@@ -53,8 +53,13 @@ impl App {
                 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 across = if self.tree.is_some() {
+                    format!(" across {files} files")
+                } else if files > 1 {
+                    format!(" across {files} replies")
+                } else {
+                    String::new()
+                };
                 let mut status = format!("{verb} {} annotation(s){across} → {target}", feedback.count);
                 if !errors.is_empty() {
                     let _ = write!(
@@ -104,7 +109,11 @@ impl App {
             {
                 self.folder_counts.insert(path.clone(), ReviewCounts::for_store(&part.store));
             }
-            if part.path.as_deref().is_none_or(|p| self.is_open(p)) {
+            if let Some(index) = part.reply {
+                if let Some(store) = self.reply_store_mut(index) {
+                    *store = part.store;
+                }
+            } else if part.path.as_deref().is_none_or(|p| self.is_open(p)) {
                 self.open.store = part.store;
             }
         }
@@ -170,7 +179,11 @@ impl App {
     }
 
     pub(super) fn send_count(&self) -> usize {
-        if self.is_file_review() { self.review_counts().pending } else { self.open.store.placed().len() }
+        if self.is_file_review() {
+            self.review_counts().pending
+        } else {
+            self.replies().iter().map(|(_, open)| open.store.placed().len()).sum()
+        }
     }
 
     pub(super) fn send_label(&self) -> String {
@@ -227,7 +240,15 @@ impl App {
             let counts = self.review_counts();
             counts.pending == 0 && counts.sent > 0
         } else {
-            self.open.store.all_delivered()
+            // Every reply with notes has had them delivered; a reply without notes has
+            // nothing to send and does not hold the review back.
+            let annotated: Vec<&Store> = self
+                .replies()
+                .into_iter()
+                .map(|(_, open)| &open.store)
+                .filter(|store| store.len() > 0)
+                .collect();
+            !annotated.is_empty() && annotated.iter().all(|store| store.all_delivered())
         };
         self.send_state = if delivered { SendState::Sent } else { SendState::Ready };
     }

crates/plannotator-tui/src/app/tests.rs

@@ -629,3 +629,110 @@ fn paging_a_one_row_pane_keeps_the_selection() {
     app.handle_event(&key(KeyCode::Char('d'), KeyModifiers::CONTROL)).expect("ctrl+d");
     assert_eq!(app.selected, selected, "a page of zero rows does not jump to the top");
 }
+
+/// A reply review over `candidates()` that records what it sends, isolated like `app`.
+fn recorded_message_app() -> (App, super::review_test_support::RecordingDelivery) {
+    let delivery = super::review_test_support::RecordingDelivery::default();
+    let app = message_app(None, Box::new(delivery.clone()));
+    (app, delivery)
+}
+
+fn keys(app: &mut App, codes: &[KeyCode]) {
+    for code in codes {
+        app.handle_event(&Event::Key(KeyEvent::from(*code))).expect("key");
+    }
+}
+
+/// Annotate the newest reply, then open the middle one from the picker.
+fn note_newest_then_open_middle(app: &mut App) {
+    keys(app, &[KeyCode::Esc]);
+    app.add_block_annotation(0, Kind::Comment, "on the newest".to_owned()).expect("annotate");
+    keys(app, &[KeyCode::Char('p'), KeyCode::Char('j'), KeyCode::Enter]);
+    assert_eq!(app.open.doc.source, "# Second\n\nmiddle message\n");
+}
+
+#[test]
+fn notes_on_every_reply_are_sent_together_each_under_its_own_reply() {
+    let (mut app, delivery) = recorded_message_app();
+    note_newest_then_open_middle(&mut app);
+    app.add_block_annotation(1, Kind::Comment, "on the middle".to_owned()).expect("annotate");
+    assert_eq!(app.send_count(), 2, "both replies' notes are waiting");
+
+    keys(&mut app, &[KeyCode::Char('E')]);
+    let calls = delivery.calls.borrow();
+    assert_eq!(calls.len(), 1);
+    assert_eq!(
+        calls[0],
+        "# Annotations on claude · message 1 of 3 (\"Third\")\n\n\
+         ## Annotation 1 (line 1)\nComment on: \"# Third\"\n> on the newest\n\n\
+         \n# Annotations on claude · message 2 of 3 (\"Second\")\n\n\
+         ## Annotation 1 (line 3)\nComment on: \"middle message\"\n> on the middle\n\n"
+    );
+    assert_eq!(app.send_state, SendState::Sent);
+    assert!(!app.has_unsent(), "every reply's notes were delivered");
+    let status = app.status.clone().expect("status");
+    assert!(status.starts_with("sent 2 annotation(s) across 2 replies"), "{status}");
+
+    let index = app.data_dir.join("feedback").join(&app.project).join("index.jsonl");
+    let text = std::fs::read_to_string(&index).expect("archived");
+    let record: serde_json::Value = serde_json::from_str(text.trim()).expect("one json record");
+    assert_eq!(record["surface"], "annotate-last");
+    assert_eq!(record["feedback"], calls[0].as_str());
+    assert_eq!(record["annotations"].as_array().expect("annotations").len(), 2);
+    drop(calls);
+
+    keys(&mut app, &[KeyCode::Char('p'), KeyCode::Char('k'), KeyCode::Enter]);
+    assert!(app.open.store.all_delivered(), "the newest reply's note was marked sent too");
+    keys(&mut app, &[KeyCode::Char('S')]);
+    assert!(app.quit, "nothing left to send, so S closes");
+    assert_eq!(delivery.calls.borrow().len(), 1, "and does not send again");
+}
+
+#[test]
+fn quitting_with_notes_only_on_a_reply_that_is_not_open_asks_first() {
+    let (mut app, delivery) = recorded_message_app();
+    note_newest_then_open_middle(&mut app);
+    assert!(app.has_unsent(), "the newest reply's note is unsent");
+    keys(&mut app, &[KeyCode::Char('q')]);
+    assert_eq!(app.mode, Mode::ConfirmQuit);
+    assert!(!app.quit);
+    keys(&mut app, &[KeyCode::Char('y')]);
+    assert!(app.quit);
+    let calls = delivery.calls.borrow();
+    assert_eq!(calls.len(), 1, "y sends the note on the reply that is not open");
+    assert!(calls[0].contains("on the newest"), "{}", calls[0]);
+}
+
+/// One reply with notes sends exactly what a review that never used the picker sends,
+/// whichever reply happens to be open.
+#[test]
+fn a_single_annotated_reply_sends_the_same_body_as_before() {
+    let expected = "# Annotations on claude · last message\n\n\
+                    ## Annotation 1 (line 1)\nComment on: \"# Third\"\n> on the newest\n\n";
+    let (mut app, delivery) = recorded_message_app();
+    keys(&mut app, &[KeyCode::Esc]);
+    app.add_block_annotation(0, Kind::Comment, "on the newest".to_owned()).expect("annotate");
+    keys(&mut app, &[KeyCode::Char('E')]);
+    assert_eq!(delivery.calls.borrow().as_slice(), [expected]);
+
+    let (mut app, delivery) = recorded_message_app();
+    note_newest_then_open_middle(&mut app);
+    keys(&mut app, &[KeyCode::Char('E')]);
+    assert_eq!(delivery.calls.borrow().as_slice(), [expected]);
+    assert_eq!(app.status.as_deref(), Some("sent 1 annotation(s) → test agent"));
+}
+
+#[test]
+fn a_reply_already_sent_is_not_sent_again_from_another_reply() {
+    let (mut app, delivery) = recorded_message_app();
+    keys(&mut app, &[KeyCode::Esc]);
+    app.add_block_annotation(0, Kind::Comment, "on the newest".to_owned()).expect("annotate");
+    keys(&mut app, &[KeyCode::Char('E')]);
+    keys(&mut app, &[KeyCode::Char('p'), KeyCode::Char('j'), KeyCode::Enter]);
+    app.add_block_annotation(1, Kind::Comment, "on the middle".to_owned()).expect("annotate");
+    keys(&mut app, &[KeyCode::Char('E')]);
+    let calls = delivery.calls.borrow();
+    assert_eq!(calls.len(), 2);
+    assert!(calls[1].contains("on the middle"), "{}", calls[1]);
+    assert!(!calls[1].contains("on the newest"), "the newest reply was sent already: {}", calls[1]);
+}