55f0002 radicle: deprecate old location in favour of DiffLocation — heartwood

Radicle Heartwood Protocol & Stack

radicle: deprecate old location in favour of DiffLocation

Fintan Halpenny committed 9 months ago

commit 55f000268fc95c670ce2ba6b78ccdab925c1ab4e
parent 0819ebfc7bc21de80f4ffa4a4ea2000fbaa91047

12 files changed +1931 -523

modified crates/radicle-cli/examples/rad-patch-delete.md

@@ -25,7 +25,7 @@ Fetching rad:z42hL2jL4XNk6K8oHQaSWfMgCL7ji from the network, found 2 potential s

 🌱 Fetched from z6Mkux1aUQD2voWWukVb5nNUR7thrHveQG4pDQua8nVhib7Z
 $ rad patch comment 6c61ef1 -m "I think we should use MIT"
 ╭───────────────────────────╮
 │ bob (you) now 833db19     │
 │ bob (you) now f20d3ec     │
 │ I think we should use MIT │
 ╰───────────────────────────╯
 ✓ Synced with 2 seed(s)

@@ -47,12 +47,12 @@ $ rad patch show 6c61ef1 -v

 ├────────────────────────────────────────────────────┤
 │ ● opened by alice (you) (717c900) now              │
 ├────────────────────────────────────────────────────┤
 │ bob z6Mkt67…v4N1tRk now 833db19                    │
 │ bob z6Mkt67…v4N1tRk now f20d3ec                    │
 │ I think we should use MIT                          │
 ╰────────────────────────────────────────────────────╯
 $ rad patch comment 6c61ef1 --reply-to 833db19 -m "Thanks, I'll add it!"
 $ rad patch comment 6c61ef1 --reply-to f20d3ec -m "Thanks, I'll add it!"
 ╭─────────────────────────╮
 │ alice (you) now 1803a38 │
 │ alice (you) now 6af0c73 │
 │ Thanks, I'll add it!    │
 ╰─────────────────────────╯
 ✓ Synced with 2 seed(s)

@@ -70,7 +70,7 @@ $ git commit -am "Add MIT License"

 ``` ~alice (stderr)
 $ git push -f
 ✓ Patch 6c61ef1 updated to revision 93915b9afa94a9dc4f52f12cdf077d4613ea3eb3
 ✓ Patch 6c61ef1 updated to revision f415ddfb8ef2c4e58b73e96cec8303941fe791d2
 To compare against your previous revision 6c61ef1, run:
    git range-diff f2de534[..] 717c900[..] 1cc8cd9[..]

@@ -98,7 +98,7 @@ $ rad patch show 6c61ef1 -v

 │ 717c900 Introduce license                                           │
 ├─────────────────────────────────────────────────────────────────────┤
 │ ● opened by alice z6MknSL…StBU8Vi (717c900) now                     │
 │ ↑ updated to 93915b9afa94a9dc4f52f12cdf077d4613ea3eb3 (1cc8cd9) now │
 │ ↑ updated to f415ddfb8ef2c4e58b73e96cec8303941fe791d2 (1cc8cd9) now │
 │   └─ ✓ accepted by bob (you) now                                    │
 ╰─────────────────────────────────────────────────────────────────────╯
 ```

@@ -124,7 +124,7 @@ $ rad patch show 6c61ef1 -v

 │ 717c900 Introduce license                                           │
 ├─────────────────────────────────────────────────────────────────────┤
 │ ● opened by alice (you) (717c900) now                               │
 │ ↑ updated to 93915b9afa94a9dc4f52f12cdf077d4613ea3eb3 (1cc8cd9) now │
 │ ↑ updated to f415ddfb8ef2c4e58b73e96cec8303941fe791d2 (1cc8cd9) now │
 ╰─────────────────────────────────────────────────────────────────────╯
 ```

modified crates/radicle-cli/examples/rad-patch.md

@@ -140,19 +140,19 @@ And let's leave a quick comment for our team:

 ```
 $ rad patch comment aa45913 --message 'I cannot wait to get back to the 90s!' --no-announce
 ╭───────────────────────────────────────╮
 │ alice (you) now 686ec1c               │
 │ alice (you) now a6202ea               │
 │ I cannot wait to get back to the 90s! │
 ╰───────────────────────────────────────╯
 $ rad patch comment aa45913 --message 'My favorite decade!' --reply-to 686ec1c -q --no-announce
 f4336e42daf76342f787d574b5ee779d89d05c7a
 $ rad patch comment aa45913 --message 'My favorite decade!' --reply-to a6202ea -q --no-announce
 c13c19202750bd9aa81fffdea2198cedfba7
 ```
 If we realize we made a mistake in the comment, we can go back and edit it:
 ```
 $ rad patch comment aa45913 --edit 686ec1c --message 'I cannot wait to get back to the 80s!' --no-announce
 $ rad patch comment aa45913 --edit a6202ea --message 'I cannot wait to get back to the 80s!' --no-announce
 ╭───────────────────────────────────────╮
 │ alice (you) now 686ec1c               │
 │ alice (you) now a6202ea               │
 │ I cannot wait to get back to the 80s! │
 ╰───────────────────────────────────────╯
 ```

@@ -160,8 +160,8 @@ $ rad patch comment aa45913 --edit 686ec1c --message 'I cannot wait to get back

 And if we really made a mistake, then we can redact the comment entirely:
 ```
 $ rad patch comment aa45913 --redact f4336e4 --no-announce
 ✓ Redacted comment f4336e42daf76342f787d574b5ee779d89d05c7a
 $ rad patch comment aa45913 --redact 9063c13c19202750bd9aa81fffdea2198cedfba7 --no-announce
 ✓ Redacted comment 9063c13c19202750bd9aa81fffdea2198cedfba7
 ```
 Now, let's checkout the patch that we just created:

modified crates/radicle-cli/examples/workflow/4-patching-contributor.md

@@ -90,6 +90,6 @@ And let's leave a quick comment for our team:

 ```
 $ rad patch comment e4934b6d9dbe01ce3c7fbb5b77a80d5f1dacdc46 --message 'I cannot wait to get back to the 90s!' -q
 c66f87afadc7c7c857f8bb92973c25f64e75776
 a8598c5f6ad9f4aa3acd701a61a380b0d275
 ✓ Synced with 1 seed(s)
 ```

modified crates/radicle-cli/examples/workflow/5-patching-maintainer.md

@@ -63,7 +63,7 @@ $ git commit -m "Use markdown for requirements"

 ```
 ``` (stderr)
 $ git push rad -o no-sync -o patch.message="Use markdown for requirements"
 ✓ Patch e4934b6 updated to revision 9d62420e779e5cfe1dc02c51eddec9a0907aa844
 ✓ Patch e4934b6 updated to revision 9e458d00b2e9a26993113c48259781725e2cbee3
 To compare against your previous revision 773b9aa, run:
    git range-diff f2de534[..] 27857ec[..] f567f69[..]

@@ -75,7 +75,7 @@ To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkE

 Great, all fixed up, lets accept and merge the code.
 ```
 $ rad patch review e4934b6 --revision 9d62420 --accept
 $ rad patch review e4934b6 --revision 9e458d00b2e9a26993113c48259781725e2cbee3 --accept
 ✓ Patch e4934b6 accepted
 ✓ Synced with 1 seed(s)
 $ git checkout master

@@ -91,7 +91,7 @@ Fast-forward

 ```
 ``` (stderr)
 $ git push rad master
 ✓ Patch e4934b6d9dbe01ce3c7fbb5b77a80d5f1dacdc46 merged at revision 9d62420
 ✓ Patch e4934b6d9dbe01ce3c7fbb5b77a80d5f1dacdc46 merged at revision 9e458d0
 ✓ Canonical reference refs/heads/master updated to target commit f567f695d25b4e8fb63b5f5ad2a584529826e908
 ✓ Synced with 1 seed(s)
 To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi

@@ -117,9 +117,9 @@ $ rad patch show e4934b6

 ├─────────────────────────────────────────────────────────────────────┤
 │ ● opened by bob z6Mkt67…v4N1tRk (3e674d1) now                       │
 │ ↑ updated to 773b9aab58b11e9fa83d0ed0baca2bea6ff889c9 (27857ec) now │
 │ * revised by alice (you) in 9d62420 (f567f69) now                   │
 │ * revised by alice (you) in 9e458d0 (f567f69) now                   │
 │   └─ ✓ accepted by alice (you) now                                  │
 │   └─ ✓ merged by alice (you) at revision 9d62420 (f567f69) now      │
 │   └─ ✓ merged by alice (you) at revision 9e458d0 (f567f69) now      │
 ╰─────────────────────────────────────────────────────────────────────╯
 ```

modified crates/radicle-cli/src/commands/patch/review/builder.rs

@@ -13,21 +13,24 @@

 //!
 use std::collections::VecDeque;
 use std::fmt::Write as _;
 use std::ops::{Deref, Not, Range};
 use std::ops::{Deref, Range};
 use std::path::{Path, PathBuf};
 use std::str::FromStr;
 use std::{fmt, io};
 use radicle::cob;
 use radicle::cob::patch::{PatchId, Revision, Verdict};
 use radicle::cob::{CodeLocation, CodeRange};
 use radicle::cob::{CodeRange, DiffLocation, HunkIndex};
 use radicle::crypto;
 use radicle::git;
 use radicle::node::device::Device;
 use radicle::prelude::*;
 use radicle::storage::git::{cob::DraftStore, Repository};
 use radicle_git_ext::Oid;
 use radicle_surf::diff::*;
 use radicle_surf::diff;
 use radicle_surf::diff::{
     Copied, Diff, DiffContent, DiffFile, EofNewLine, FileDiff, Hunks, Modification, Moved,
 };
 use radicle_term::{Element, VStack};
 use crate::git::pretty_diff::ToPretty;

@@ -136,6 +139,33 @@ impl FromStr for ReviewAction {

     }
 }
 /// Keep track of the [`diff::Hunk`] and its `index` within the diff patch.
 #[derive(Debug)]
 pub struct Hunk {
     /// Index of the hunk within its respective patch.
     index: usize,
     /// The [`diff::Hunk`] that is being kept track of.
     inner: diff::Hunk<Modification>,
 }
 impl Hunk {
     pub fn new(index: usize, hunk: diff::Hunk<Modification>) -> Self {
         Self { index, inner: hunk }
     }
     pub fn as_index(&self, range: Option<Range<usize>>) -> Option<HunkIndex> {
         range.map(|range| HunkIndex::new(self.index, CodeRange::lines(range)))
     }
 }
 impl TryFrom<&Hunk> for HunkHeader {
     type Error = unified_diff::Error;
     fn try_from(Hunk { ref inner, .. }: &Hunk) -> Result<Self, Self::Error> {
         Self::try_from(inner)
     }
 }
 /// A single review item. Can be a hunk or eg. a file move.
 /// Files are usually split into multiple review items.
 #[derive(Debug)]

@@ -144,20 +174,20 @@ pub enum ReviewItem {

         path: PathBuf,
         header: FileHeader,
         new: DiffFile,
         hunk: Option<Hunk<Modification>>,
         hunk: Option<Hunk>,
     },
     FileDeleted {
         path: PathBuf,
         header: FileHeader,
         old: DiffFile,
         hunk: Option<Hunk<Modification>>,
         hunk: Option<Hunk>,
     },
     FileModified {
         path: PathBuf,
         header: FileHeader,
         old: DiffFile,
         new: DiffFile,
         hunk: Option<Hunk<Modification>>,
         hunk: Option<Hunk>,
     },
     FileMoved {
         moved: Moved,

@@ -181,7 +211,7 @@ pub enum ReviewItem {

 }
 impl ReviewItem {
     fn hunk(&self) -> Option<&Hunk<Modification>> {
     fn hunk(&self) -> Option<&Hunk> {
         match self {
             Self::FileAdded { hunk, .. } => hunk.as_ref(),
             Self::FileDeleted { hunk, .. } => hunk.as_ref(),

@@ -260,7 +290,7 @@ impl ReviewItem {

                     .child(header);
                 if let Some(hunk) = hunk {
                     let hunk = hunk.pretty(&mut hi, &highlighted, repo);
                     let hunk = hunk.inner.pretty(&mut hi, &highlighted, repo);
                     if !hunk.is_empty() {
                         return vstack.divider().merge(hunk).boxed();
                     }

@@ -312,7 +342,9 @@ impl ReviewQueue {

                         ..
                     } = a.diff
                     {
                         hs.pop()
                         hs.len()
                             .checked_sub(1)
                             .and_then(|i| hs.pop().map(|h| Hunk::new(i, h)))
                     } else {
                         None
                     },

@@ -328,7 +360,9 @@ impl ReviewQueue {

                         ..
                     } = d.diff
                     {
                         hs.pop()
                         hs.len()
                             .checked_sub(1)
                             .and_then(|i| hs.pop().map(|h| Hunk::new(i, h)))
                     } else {
                         None
                     },

@@ -361,13 +395,13 @@ impl ReviewQueue {

                         eof,
                         ..
                     } => {
                         for hunk in hunks {
                         for (i, hunk) in hunks.iter().enumerate() {
                             self.add_item(ReviewItem::FileModified {
                                 path: m.path.clone(),
                                 header: header.clone(),
                                 old: m.old.clone(),
                                 new: m.new.clone(),
                                 hunk: Some(hunk),
                                 hunk: Some(Hunk::new(i, hunk.clone())),
                             });
                         }
                         if let EofNewLine::OldMissing | EofNewLine::NewMissing = eof {

@@ -461,11 +495,12 @@ impl FileReviewBuilder {

             header.old_line_no -= self.delta as u32;
             header.new_line_no -= self.delta as u32;
             let h = Hunk {
             let h = h.inner.clone();
             let h = diff::Hunk {
                 header: header.to_unified_string()?.as_bytes().to_owned().into(),
                 lines: h.lines.clone(),
                 old: h.old.clone(),
                 new: h.new.clone(),
                 lines: h.lines,
                 old: h.old,
                 new: h.new,
             };
             writer.encode(&h)?;
         }

@@ -710,7 +745,11 @@ impl<'a> ReviewBuilder<'a> {

                     let path = old.or(new);
                     if let (Some(hunk), Some((path, _))) = (item.hunk(), path) {
                         let builder = CommentBuilder::new(revision.head(), path.to_path_buf());
                         let builder = CommentBuilder::new(
                             *revision.base(),
                             revision.head(),
                             path.to_path_buf(),
                         );
                         let comments = builder.edit(hunk)?;
                         patch.transaction("Review comments", signer, |tx| {

@@ -820,7 +859,7 @@ impl<'a> ReviewBuilder<'a> {

 #[derive(Debug, PartialEq, Eq)]
 struct ReviewComment {
     location: CodeLocation,
     location: DiffLocation,
     body: String,
 }

@@ -840,23 +879,25 @@ enum Error {

 #[derive(Debug)]
 struct CommentBuilder {
     base: Oid,
     commit: Oid,
     path: PathBuf,
     comments: Vec<ReviewComment>,
 }
 impl CommentBuilder {
     fn new(commit: Oid, path: PathBuf) -> Self {
     fn new(base: Oid, commit: Oid, path: PathBuf) -> Self {
         Self {
             base,
             commit,
             path,
             comments: Vec::new(),
         }
     }
     fn edit(mut self, hunk: &Hunk<Modification>) -> Result<Vec<ReviewComment>, Error> {
     fn edit(mut self, hunk: &Hunk) -> Result<Vec<ReviewComment>, Error> {
         let mut input = String::new();
         for line in hunk.to_unified_string()?.lines() {
         for line in hunk.inner.to_unified_string()?.lines() {
             writeln!(&mut input, "> {line}")?;
         }
         let output = term::Editor::comment()

@@ -865,91 +906,65 @@ impl CommentBuilder {

             .edit()?;
         if let Some(output) = output {
             let header = HunkHeader::try_from(hunk)?;
             self.add_hunk(header, &output);
             self.add_hunk(hunk, &output);
         }
         Ok(self.comments())
     }
     fn add_hunk(&mut self, hunk: HunkHeader, input: &str) -> &mut Self {
         let lines = input.trim().lines().map(|l| l.trim());
         let (mut old_line, mut new_line) = (hunk.old_line_no as usize, hunk.new_line_no as usize);
         let (mut old_start, mut new_start) = (old_line, new_line);
     fn add_hunk(&mut self, hunk: &Hunk, input: &str) -> &mut Self {
         let lines = input
             .trim()
             .lines()
             .map(|l| l.trim())
             // Skip the hunk header
             .filter(|l| !l.starts_with("> @@"));
         let mut comment = String::new();
         // Keeps track of where the next range will start from
         let mut range_start = 0;
         // Keeps track of the line index within the hunk itself
         let mut line_ix = 0;
         // Keeps track of whether the first comment is at the top-level
         let mut top_level = true;
         for line in lines {
             if line.starts_with('>') {
                 if !comment.is_empty() {
                     self.add_comment(
                         &hunk,
                         &comment,
                         old_start..old_line - 1,
                         new_start..new_line - 1,
                     );
                     old_start = old_line - 1;
                     new_start = new_line - 1;
                     comment.clear();
                 }
                 match line.trim_start_matches('>').trim_start().chars().next() {
                     Some('-') => old_line += 1,
                     Some('+') => new_line += 1,
                     _ => {
                         old_line += 1;
                         new_line += 1;
                     if top_level {
                         // Top-level comment
                         self.add_comment(hunk.as_index(None), &comment);
                     } else {
                         self.add_comment(hunk.as_index(Some(range_start..line_ix)), &comment);
                     }
                     range_start = line_ix;
                     comment.clear();
                 }
                 line_ix += 1;
                 // Can no longer be a top-level comment
                 top_level = false;
             } else {
                 comment.push_str(line);
                 comment.push('\n');
             }
         }
         if !comment.is_empty() {
             self.add_comment(
                 &hunk,
                 &comment,
                 old_start..old_line - 1,
                 new_start..new_line - 1,
             );
             self.add_comment(hunk.as_index(Some(range_start..line_ix)), &comment);
         }
         self
     }
     fn add_comment(
         &mut self,
         hunk: &HunkHeader,
         comment: &str,
         mut old_range: Range<usize>,
         mut new_range: Range<usize>,
     ) {
     fn add_comment(&mut self, selection: Option<HunkIndex>, comment: &str) {
         // Empty lines between quoted text can generate empty comments
         // that should be filtered out.
         if comment.trim().is_empty() {
             return;
         }
         // Top-level comment, it should apply to the whole hunk.
         if old_range.is_empty() && new_range.is_empty() {
             old_range = hunk.old_line_no as usize..(hunk.old_line_no + hunk.old_size + 1) as usize;
             new_range = hunk.new_line_no as usize..(hunk.new_line_no + hunk.new_size + 1) as usize;
         }
         let old_range = old_range
             .is_empty()
             .not()
             .then_some(old_range)
             .map(|range| CodeRange::Lines { range });
         let new_range = (new_range)
             .is_empty()
             .not()
             .then_some(new_range)
             .map(|range| CodeRange::Lines { range });
         self.comments.push(ReviewComment {
             location: CodeLocation {
                 commit: self.commit,
             location: DiffLocation {
                 base: self.base,
                 head: self.commit,
                 path: self.path.clone(),
                 old: old_range,
                 new: new_range,
                 selection,
             },
             body: comment.trim().to_owned(),
         });

@@ -1010,65 +1025,73 @@ Comment #5.

 "#;
         let base = git::raw::Oid::zero().into();
         let commit = Oid::from_str("a32c4b93e2573fd83b15ac1ad6bf1317dc8fd760").unwrap();
         let path = PathBuf::from_str("main.rs").unwrap();
         let expected = &[
             (ReviewComment {
                 location: CodeLocation {
                 location: DiffLocation::hunk_level(
                     base,
                     commit,
                     path: path.clone(),
                     old: Some(CodeRange::Lines { range: 2559..2565 }),
                     new: Some(CodeRange::Lines { range: 2560..2563 }),
                 },
                     path.clone(),
                     HunkIndex::new(0, CodeRange::lines(0..6)),
                 ),
                 body: "Comment #1.".to_owned(),
             }),
             (ReviewComment {
                 location: CodeLocation {
                 location: DiffLocation::hunk_level(
                     base,
                     commit,
                     path: path.clone(),
                     old: Some(CodeRange::Lines { range: 2565..2568 }),
                     new: Some(CodeRange::Lines { range: 2563..2567 }),
                 },
                     path.clone(),
                     HunkIndex::new(0, CodeRange::lines(6..12)),
                 ),
                 body: "Comment #2.".to_owned(),
             }),
             (ReviewComment {
                 location: CodeLocation {
                 location: DiffLocation::hunk_level(
                     base,
                     commit,
                     path: path.clone(),
                     old: Some(CodeRange::Lines { range: 2568..2571 }),
                     new: Some(CodeRange::Lines { range: 2567..2570 }),
                 },
                     path.clone(),
                     HunkIndex::new(0, CodeRange::lines(12..17)),
                 ),
                 body: "Comment #3.".to_owned(),
             }),
             (ReviewComment {
                 location: CodeLocation {
                 location: DiffLocation::hunk_level(
                     base,
                     commit,
                     path: path.clone(),
                     old: None,
                     new: Some(CodeRange::Lines { range: 2570..2571 }),
                 },
                     path.clone(),
                     HunkIndex::new(0, CodeRange::lines(17..18)),
                 ),
                 body: "Comment #4.".to_owned(),
             }),
             (ReviewComment {
                 location: CodeLocation {
                 location: DiffLocation::hunk_level(
                     base,
                     commit,
                     path: path.clone(),
                     old: Some(CodeRange::Lines { range: 2571..2577 }),
                     new: Some(CodeRange::Lines { range: 2571..2578 }),
                 },
                     path.clone(),
                     HunkIndex::new(0, CodeRange::lines(18..26)),
                 ),
                 body: "Comment #5.".to_owned(),
             }),
         ];
         let mut builder = CommentBuilder::new(commit, path.clone());
         let mut builder = CommentBuilder::new(base, commit, path.clone());
         builder.add_hunk(
             HunkHeader {
                 old_line_no: 2559,
                 old_size: 18,
                 new_line_no: 2560,
                 new_size: 18,
                 text: vec![],
             },
             &Hunk::new(
 ,
                 diff::Hunk {
                     header: diff::Line::from(vec![]),
                     lines: std::iter::repeat(diff::Modification::addition(
                         diff::Line::from(vec![]),
 ,
                     ))
                     .take(26)
                     .collect(),
                     old: 2559..2578,
                     new: 2560..2579,
                 },
             ),
             input,
         );
         let actual = builder.comments();

@@ -1114,16 +1137,17 @@ Woof.

 "#;
         let base = git::raw::Oid::zero().into();
         let commit = Oid::from_str("a32c4b93e2573fd83b15ac1ad6bf1317dc8fd760").unwrap();
         let path = PathBuf::from_str("main.rs").unwrap();
         let expected = &[
             (ReviewComment {
                 location: CodeLocation {
                 location: DiffLocation::hunk_level(
                     base,
                     commit,
                     path: path.clone(),
                     old: Some(CodeRange::Lines { range: 2559..2565 }),
                     new: Some(CodeRange::Lines { range: 2560..2563 }),
                 },
                     path.clone(),
                     HunkIndex::new(0, CodeRange::lines(0..6)),
                 ),
                 body: r#"
 Blah blah blah blah blah blah blah.
 Blah blah blah.

@@ -1137,12 +1161,12 @@ Blaaah blaaah blaaah.

                 .to_owned(),
             }),
             (ReviewComment {
                 location: CodeLocation {
                 location: DiffLocation::hunk_level(
                     base,
                     commit,
                     path: path.clone(),
                     old: Some(CodeRange::Lines { range: 2565..2568 }),
                     new: Some(CodeRange::Lines { range: 2563..2567 }),
                 },
                     path.clone(),
                     HunkIndex::new(0, CodeRange::lines(6..12)),
                 ),
                 body: r#"
 Woof woof.
 Woof.

@@ -1155,15 +1179,22 @@ Woof.

             }),
         ];
         let mut builder = CommentBuilder::new(commit, path.clone());
         let mut builder = CommentBuilder::new(base, commit, path.clone());
         builder.add_hunk(
             HunkHeader {
                 old_line_no: 2559,
                 old_size: 9,
                 new_line_no: 2560,
                 new_size: 7,
                 text: vec![],
             },
             &Hunk::new(
 ,
                 diff::Hunk {
                     header: diff::Line::from(vec![]),
                     lines: std::iter::repeat(diff::Modification::addition(
                         diff::Line::from(vec![]),
 ,
                     ))
                     .take(12)
                     .collect(),
                     old: 2559..2569,
                     new: 2560..2568,
                 },
             ),
             input,
         );
         let actual = builder.comments();

@@ -1195,27 +1226,30 @@ This is a top-level comment.

 > +        let connect = available.take(wanted).collect::<Vec<_>>();
 "#;
         let base = git::raw::Oid::zero().into();
         let commit = Oid::from_str("a32c4b93e2573fd83b15ac1ad6bf1317dc8fd760").unwrap();
         let path = PathBuf::from_str("main.rs").unwrap();
         let expected = &[(ReviewComment {
             location: CodeLocation {
                 commit,
                 path: path.clone(),
                 old: Some(CodeRange::Lines { range: 2559..2569 }),
                 new: Some(CodeRange::Lines { range: 2560..2568 }),
             },
             location: DiffLocation::file_level(git::raw::Oid::zero().into(), commit, path.clone()),
             body: "This is a top-level comment.".to_owned(),
         })];
         let mut builder = CommentBuilder::new(commit, path.clone());
         let mut builder = CommentBuilder::new(base, commit, path.clone());
         builder.add_hunk(
             HunkHeader {
                 old_line_no: 2559,
                 old_size: 9,
                 new_line_no: 2560,
                 new_size: 7,
                 text: vec![],
             },
             &Hunk::new(
 ,
                 diff::Hunk {
                     header: diff::Line::from(vec![]),
                     lines: std::iter::repeat(diff::Modification::addition(
                         diff::Line::from(vec![]),
 ,
                     ))
                     .take(12)
                     .collect(),
                     old: 2559..2569,
                     new: 2560..2568,
                 },
             ),
             input,
         );
         let actual = builder.comments();

@@ -1243,27 +1277,30 @@ This is a top-level comment.

 Comment on a split hunk.
 "#;
         let base = git::raw::Oid::zero().into();
         let commit = Oid::from_str("a32c4b93e2573fd83b15ac1ad6bf1317dc8fd760").unwrap();
         let path = PathBuf::from_str("main.rs").unwrap();
         let expected = &[(ReviewComment {
             location: CodeLocation {
             location: DiffLocation::hunk_level(
                 base,
                 commit,
                 path: path.clone(),
                 old: Some(CodeRange::Lines { range: 2564..2565 }),
                 new: Some(CodeRange::Lines { range: 2563..2564 }),
             },
                 path.clone(),
                 HunkIndex::new(0, CodeRange::lines(5..7)),
             ),
             body: "Comment on a split hunk.".to_owned(),
         })];
         let mut builder = CommentBuilder::new(commit, path.clone());
         let mut builder = CommentBuilder::new(base, commit, path.clone());
         builder.add_hunk(
             HunkHeader {
                 old_line_no: 2559,
                 old_size: 6,
                 new_line_no: 2560,
                 new_size: 4,
                 text: vec![],
             },
             &Hunk::new(
 ,
                 diff::Hunk {
                     header: diff::Line::from(vec![]),
                     lines: vec![],
                     old: 2559..2566,
                     new: 2560..2565,
                 },
             ),
             input,
         );
         let actual = builder.comments();

modified crates/radicle/src/cob/common.rs

@@ -1,5 +1,6 @@

 use std::fmt;
 use std::fmt::Display;
 use std::hash::Hash;
 use std::ops::{Deref, Range};
 use std::path::PathBuf;
 use std::str::FromStr;

@@ -373,60 +374,85 @@ impl From<bool> for Authorization {

     }
 }
 /// **Note**: this type is deprecated and should no longer be used.
 /// Use [`DiffLocation`] instead.
 ///
 /// Describes a code location that can be used for comments on
 /// patches, issues, and diffs.
 ///
 /// It is called a `PartialLocation`, because it describes a potential diff
 /// compared to its `commit`, but does not describe the other `Oid` – thus the
 /// other `Oid` must be inferred from context.
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct CodeLocation {
 pub struct PartialLocation {
     /// [`Oid`] of the Git commit.
     pub commit: Oid,
     pub(crate) commit: Oid,
     /// Path of file.
     pub path: PathBuf,
     pub(crate) path: PathBuf,
     /// Line range on old file. `None` for added files.
     pub old: Option<CodeRange>,
     pub(crate) old: Option<CodeRange>,
     /// Line range on new file. `None` for deleted files.
     pub new: Option<CodeRange>,
     pub(crate) new: Option<CodeRange>,
 }
 /// Code range.
 /// The specified range that selects a section of code.
 ///
 /// When used with [`HunkIndex`], the range represents the lines within the hunk
 /// itself, which are 0-indexed.
 ///
 /// Note: `CodeRange` is an `enum` that has the single variant `Lines`, but may
 /// add more cases in the future.
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase", tag = "type")]
 pub enum CodeRange {
     /// One or more lines.
     Lines { range: Range<usize> },
     /// Character range within a line.
     Chars { line: usize, range: Range<usize> },
 }
 impl std::cmp::PartialOrd for CodeRange {
 impl PartialOrd for CodeRange {
     fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
         Some(self.cmp(other))
     }
 }
 impl std::cmp::Ord for CodeRange {
 impl Ord for CodeRange {
     fn cmp(&self, other: &Self) -> std::cmp::Ordering {
         match (self, other) {
             (CodeRange::Lines { .. }, CodeRange::Chars { .. }) => std::cmp::Ordering::Less,
             (CodeRange::Chars { .. }, CodeRange::Lines { .. }) => std::cmp::Ordering::Greater,
             (CodeRange::Lines { range: a }, CodeRange::Lines { range: b }) => {
                 a.clone().cmp(b.clone())
             (CodeRange::Lines { range: x }, CodeRange::Lines { range: y }) => {
                 x.start.cmp(&y.start).then(x.end.cmp(&y.end))
             }
             (
                 CodeRange::Chars {
                     line: l1,
                     range: r1,
                 },
                 CodeRange::Chars {
                     line: l2,
                     range: r2,
                 },
             ) => l1.cmp(l2).then(r1.clone().cmp(r2.clone())),
         }
     }
 }
 impl CodeRange {
     /// Get the starting position of the range.
     ///
     /// If the range is [`CodeRange::Lines`], this corresponds to the start of
     /// the line range.
     pub fn line_start(&self) -> usize {
         match self {
             CodeRange::Lines { range } => range.start,
         }
     }
     /// Get the ending position of the range.
     ///
     /// If the range is [`CodeRange::Lines`], this corresponds to the end of
     /// the line range.
     pub fn line_end(&self) -> usize {
         match self {
             CodeRange::Lines { range } => range.end,
         }
     }
     /// Construct a `CodeRange` for which is one or more lines.
     pub fn lines(range: Range<usize>) -> Self {
         Self::Lines { range }
     }
 }
 /// Describes a selection within a diff.
 #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]

@@ -443,6 +469,8 @@ pub struct DiffLocation {

 }
 impl DiffLocation {
     /// Construct a file-level diff location, where the
     /// [`DiffLocation::selection`] is `None`.
     pub fn file_level(base: Oid, head: Oid, path: PathBuf) -> Self {
         Self {
             base,

@@ -452,6 +480,8 @@ impl DiffLocation {

         }
     }
     /// Construct a hunk-level diff location, where the
     /// [`DiffLocation::selection`] is set to the given `hunk`.
     pub fn hunk_level(base: Oid, head: Oid, path: PathBuf, hunk: HunkIndex) -> Self {
         Self {
             base,

@@ -461,13 +491,20 @@ impl DiffLocation {

         }
     }
     pub fn code_range(&self) -> Option<&CodeRange> {
         self.selection().map(|s| s.range())
     }
     /// Get the [`DiffLocation::selection`].
     pub fn selection(&self) -> Option<&HunkIndex> {
         self.selection.as_ref()
     }
     /// Get the index for the hunk of the [`DiffLocation::selection`].
     pub fn hunk_index(&self) -> Option<usize> {
         self.selection().map(|s| s.index())
     }
     /// Get the [`CodeRange`] of the [`DiffLocation::selection`].
     pub fn code_range(&self) -> Option<&CodeRange> {
         self.selection().map(|s| s.range())
     }
 }
 /// The combined index of the hunk and range within that hunk.

@@ -475,9 +512,9 @@ impl DiffLocation {

 #[serde(rename_all = "camelCase")]
 pub struct HunkIndex {
     /// The index of the hunk in the patch.
     pub hunk: usize,
     hunk: usize,
     /// The line selection within the hunk itself.
     pub range: CodeRange,
     range: CodeRange,
 }
 impl HunkIndex {

modified crates/radicle/src/cob/patch.rs

@@ -2,7 +2,7 @@ pub mod cache;

 pub mod diff;
 mod actions;
 pub use actions::ReviewEdit;
 pub use actions::{ReviewComment, ReviewEdit};
 mod encoding;

@@ -20,7 +20,7 @@ use storage::{HasRepoId, RepositoryError};

 use thiserror::Error;
 use crate::cob;
 use crate::cob::common::{Author, Authorization, CodeLocation, Label, Reaction, Timestamp};
 use crate::cob::common::{Author, Authorization, DiffLocation, Label, Reaction, Timestamp};
 use crate::cob::store::Transaction;
 use crate::cob::store::{Cob, CobAction};
 use crate::cob::thread;

@@ -195,21 +195,6 @@ pub enum Action {

     },
     #[serde(rename = "review.redact")]
     ReviewRedact { review: ReviewId },
     #[serde(rename = "review.comment")]
     ReviewComment {
         review: ReviewId,
         body: String,
         #[serde(default, skip_serializing_if = "Option::is_none")]
         location: Option<CodeLocation>,
         /// Comment this is a reply to.
         /// Should be [`None`] if it's the first comment.
         /// Should be [`Some`] otherwise.
         #[serde(default, skip_serializing_if = "Option::is_none")]
         reply_to: Option<CommentId>,
         /// Embeded content.
         #[serde(default, skip_serializing_if = "Vec::is_empty")]
         embeds: Vec<Embed<Uri>>,
     },
     #[serde(rename = "review.comment.edit")]
     ReviewCommentEdit {
         review: ReviewId,

@@ -258,36 +243,8 @@ pub enum Action {

         #[serde(default, skip_serializing_if = "Vec::is_empty")]
         embeds: Vec<Embed<Uri>>,
     },
     /// React to the revision.
     #[serde(rename = "revision.react")]
     RevisionReact {
         revision: RevisionId,
         #[serde(default, skip_serializing_if = "Option::is_none")]
         location: Option<CodeLocation>,
         reaction: Reaction,
         active: bool,
     },
     #[serde(rename = "revision.redact")]
     RevisionRedact { revision: RevisionId },
     #[serde(rename_all = "camelCase")]
     #[serde(rename = "revision.comment")]
     RevisionComment {
         /// The revision to comment on.
         revision: RevisionId,
         /// For comments on the revision code.
         #[serde(default, skip_serializing_if = "Option::is_none")]
         location: Option<CodeLocation>,
         /// Comment body.
         body: String,
         /// Comment this is a reply to.
         /// Should be [`None`] if it's the top-level comment.
         /// Should be the root [`CommentId`] if it's a top-level comment.
         #[serde(default, skip_serializing_if = "Option::is_none")]
         reply_to: Option<CommentId>,
         /// Embeded content.
         #[serde(default, skip_serializing_if = "Vec::is_empty")]
         embeds: Vec<Embed<Uri>>,
     },
     /// Edit a revision comment.
     #[serde(rename = "revision.comment.edit")]
     RevisionCommentEdit {

@@ -310,12 +267,40 @@ pub enum Action {

         reaction: Reaction,
         active: bool,
     },
     /// Edit a review's summary, verdict, labels, and embeds.
     // Note that the tags live on `actions::ReviewEdit`, and according to the
     // serde.rs docs, it must come after the other variants due to the
     // `untagged` declaration.
     /// Edit a review's summary, verdict, labels, and embeds.
     #[serde(untagged)]
     ReviewEdit(actions::ReviewEdit),
     /// Publish a comment to a review.
     #[serde(untagged)]
     ReviewComment(actions::ReviewComment),
     /// Publish a comment to a revision.
     #[serde(untagged)]
     RevisionComment(actions::RevisionComment),
     /// React to the revision.
     #[serde(untagged)]
     RevisionReact(actions::RevisionReact),
 }
 impl From<actions::ReviewComment> for Action {
     fn from(value: actions::ReviewComment) -> Self {
         Action::ReviewComment(value)
     }
 }
 impl From<actions::RevisionComment> for Action {
     fn from(value: actions::RevisionComment) -> Self {
         Action::RevisionComment(value)
     }
 }
 impl From<actions::RevisionReact> for Action {
     fn from(value: actions::RevisionReact) -> Self {
         Action::RevisionReact(value)
     }
 }
 impl CobAction for Action {

@@ -877,23 +862,7 @@ impl Patch {

                     )),
                 );
             }
             Action::RevisionReact {
                 revision,
                 reaction,
                 active,
                 location,
             } => {
                 if let Some(revision) = lookup::revision_mut(self, &revision)? {
                     let key = (author, reaction);
                     let reactions = revision.reactions.entry(location).or_default();
                     if active {
                         reactions.insert(key);
                     } else {
                         reactions.remove(&key);
                     }
                 }
             }
             Action::RevisionReact(react) => react.run(author, self)?,
             Action::RevisionRedact { revision } => {
                 // Not allowed to delete the root revision.
                 let (root, _) = self.root();

@@ -998,26 +967,7 @@ impl Patch {

                     thread::unresolve(&mut review.comments, entry, comment)?;
                 }
             }
             Action::ReviewComment {
                 review,
                 body,
                 location,
                 reply_to,
                 embeds,
             } => {
                 if let Some(review) = lookup::review_mut(self, &review)? {
                     thread::comment(
                         &mut review.comments,
                         entry,
                         author,
                         timestamp,
                         body,
                         reply_to,
                         location,
                         embeds,
                     )?;
                 }
             }
             Action::ReviewComment(action) => action.run(entry, author, timestamp, self)?,
             Action::ReviewRedact { review } => {
                 // Redactions must have observed a review to be valid.
                 let Some(locator) = self.reviews.get_mut(&review) else {

@@ -1122,26 +1072,7 @@ impl Patch {

                 }
             }
             Action::RevisionComment {
                 revision,
                 body,
                 reply_to,
                 embeds,
                 location,
             } => {
                 if let Some(revision) = lookup::revision_mut(self, &revision)? {
                     thread::comment(
                         &mut revision.discussion,
                         entry,
                         author,
                         timestamp,
                         body,
                         reply_to,
                         location,
                         embeds,
                     )?;
                 }
             }
             Action::RevisionComment(comment) => comment.run(entry, author, timestamp, self)?,
             Action::RevisionCommentEdit {
                 revision,
                 comment,

@@ -1391,7 +1322,7 @@ mod lookup {

 /// A patch revision.
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 #[serde(rename_all = "camelCase", from = "encoding::revision::Revision")]
 pub struct Revision {
     /// Revision identifier.
     pub(super) id: RevisionId,

@@ -1404,7 +1335,7 @@ pub struct Revision {

     /// Reference to the Git object containing the code (revision head).
     pub(super) oid: git::Oid,
     /// Discussion around this revision.
     pub(super) discussion: Thread<Comment<CodeLocation>>,
     pub(super) discussion: Thread<Comment<DiffLocation>>,
     /// Reviews of this revision's changes (all review edits are kept).
     pub(super) reviews: BTreeMap<ActorId, Review>,
     /// When this revision was created.

@@ -1412,11 +1343,8 @@ pub struct Revision {

     /// Review comments resolved by this revision.
     pub(super) resolves: BTreeSet<(EntryId, CommentId)>,
     /// Reactions on code locations and revision itself
     #[serde(
         serialize_with = "ser::serialize_reactions",
         deserialize_with = "ser::deserialize_reactions"
     )]
     pub(super) reactions: BTreeMap<Option<CodeLocation>, Reactions>,
     #[serde(serialize_with = "encoding::ser::serialize_reactions")]
     pub(super) reactions: BTreeMap<Option<DiffLocation>, Reactions>,
 }
 impl Revision {

@@ -1461,7 +1389,7 @@ impl Revision {

         &self.description.last().embeds
     }
     pub fn reactions(&self) -> &BTreeMap<Option<CodeLocation>, BTreeSet<(PublicKey, Reaction)>> {
     pub fn reactions(&self) -> &BTreeMap<Option<DiffLocation>, BTreeSet<(PublicKey, Reaction)>> {
         &self.reactions
     }

@@ -1491,7 +1419,7 @@ impl Revision {

     }
     /// Discussion around this revision.
     pub fn discussion(&self) -> &Thread<Comment<CodeLocation>> {
     pub fn discussion(&self) -> &Thread<Comment<DiffLocation>> {
         &self.discussion
     }

@@ -1501,7 +1429,7 @@ impl Revision {

     }
     /// Iterate over all top-level replies.
     pub fn replies(&self) -> impl Iterator<Item = (&CommentId, &thread::Comment<CodeLocation>)> {
     pub fn replies(&self) -> impl Iterator<Item = (&CommentId, &thread::Comment<DiffLocation>)> {
         self.discussion.comments()
     }

@@ -1628,9 +1556,8 @@ impl fmt::Display for Verdict {

 }
 /// A patch review on a revision.
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 #[derive(Debug, Clone, PartialEq, Eq, Serialize)]
 #[serde(rename_all = "camelCase")]
 #[serde(from = "encoding::review::Review")]
 pub struct Review {
     /// Review identifier.
     pub(super) id: ReviewId,

@@ -1648,7 +1575,7 @@ pub struct Review {

     /// edit of the summary.
     pub(super) summary: NonEmpty<Edit>,
     /// Review comments.
     pub(super) comments: Thread<Comment<CodeLocation>>,
     pub(super) comments: Thread<Comment<DiffLocation>>,
     /// Labels qualifying the review. For example if this review only looks at the
     /// concept or intention of the patch, it could have a "concept" label.
     pub(super) labels: Vec<Label>,

@@ -1698,7 +1625,7 @@ impl Review {

     }
     /// Review inline code comments.
     pub fn comments(&self) -> impl DoubleEndedIterator<Item = (&EntryId, &Comment<CodeLocation>)> {
     pub fn comments(&self) -> impl DoubleEndedIterator<Item = (&EntryId, &Comment<DiffLocation>)> {
         self.comments.comments()
     }

@@ -1766,13 +1693,13 @@ impl<R: ReadRepository> store::Transaction<Patch, R> {

         revision: RevisionId,
         body: S,
     ) -> Result<(), store::Error> {
         self.push(Action::RevisionComment {
         self.push(Action::RevisionComment(actions::RevisionComment::new(
             revision,
             body: body.to_string(),
             reply_to: None,
             location: None,
             embeds: vec![],
         })
             None,
             body.to_string(),
             None,
             vec![],
         )))
     }
     /// React on a patch revision.

@@ -1780,15 +1707,12 @@ impl<R: ReadRepository> store::Transaction<Patch, R> {

         &mut self,
         revision: RevisionId,
         reaction: Reaction,
         location: Option<CodeLocation>,
         location: Option<DiffLocation>,
         active: bool,
     ) -> Result<(), store::Error> {
         self.push(Action::RevisionReact {
             revision,
             reaction,
             location,
             active,
         })
         self.push(Action::RevisionReact(actions::RevisionReact::new(
             revision, location, reaction, active,
         )))
     }
     /// Comment on a patch revision.

@@ -1797,17 +1721,17 @@ impl<R: ReadRepository> store::Transaction<Patch, R> {

         revision: RevisionId,
         body: S,
         reply_to: Option<CommentId>,
         location: Option<CodeLocation>,
         location: Option<DiffLocation>,
         embeds: Vec<Embed<Uri>>,
     ) -> Result<(), store::Error> {
         self.embed(embeds.clone())?;
         self.push(Action::RevisionComment {
         self.push(Action::RevisionComment(actions::RevisionComment::new(
             revision,
             body: body.to_string(),
             reply_to,
             location,
             body.to_string(),
             reply_to,
             embeds,
         })
         )))
     }
     /// Edit a comment on a patch revision.

@@ -1857,18 +1781,18 @@ impl<R: ReadRepository> store::Transaction<Patch, R> {

         &mut self,
         review: ReviewId,
         body: S,
         location: Option<CodeLocation>,
         location: Option<DiffLocation>,
         reply_to: Option<CommentId>,
         embeds: Vec<Embed<Uri>>,
     ) -> Result<(), store::Error> {
         self.embed(embeds.clone())?;
         self.push(Action::ReviewComment {
         self.push(Action::ReviewComment(actions::ReviewComment::new(
             review,
             body: body.to_string(),
             body.to_string(),
             location,
             reply_to,
             embeds,
         })
         )))
     }
     /// Resolve a review comment.

@@ -2143,7 +2067,7 @@ where

         revision: RevisionId,
         body: S,
         reply_to: Option<CommentId>,
         location: Option<CodeLocation>,
         location: Option<DiffLocation>,
         embeds: impl IntoIterator<Item = Embed<Uri>>,
         signer: &Device<G>,
     ) -> Result<EntryId, Error>

@@ -2167,7 +2091,7 @@ where

         &mut self,
         revision: RevisionId,
         reaction: Reaction,
         location: Option<CodeLocation>,
         location: Option<DiffLocation>,
         active: bool,
         signer: &Device<G>,
     ) -> Result<EntryId, Error>

@@ -2234,7 +2158,7 @@ where

         &mut self,
         review: ReviewId,
         body: S,
         location: Option<CodeLocation>,
         location: Option<DiffLocation>,
         reply_to: Option<CommentId>,
         embeds: impl IntoIterator<Item = Embed<Uri>>,
         signer: &Device<G>,

@@ -2847,129 +2771,10 @@ impl From<RangeDiff> for std::process::Command {

     }
 }
 /// Helpers for de/serialization of patch data types.
 mod ser {
     use std::collections::{BTreeMap, BTreeSet};
     use serde::ser::SerializeSeq;
     use crate::cob::{thread::Reactions, ActorId, CodeLocation};
     /// Serialize a `Revision`'s reaction as an object containing the
     /// `location`, `emoji`, and all `authors` that have performed the
     /// same reaction.
     #[derive(Debug, serde::Serialize, serde::Deserialize)]
     #[serde(rename_all = "camelCase")]
     struct Reaction {
         location: Option<CodeLocation>,
         emoji: super::Reaction,
         authors: Vec<ActorId>,
     }
     impl Reaction {
         fn as_revision_reactions(
             reactions: Vec<Reaction>,
         ) -> BTreeMap<Option<CodeLocation>, Reactions> {
             reactions.into_iter().fold(
                 BTreeMap::<Option<CodeLocation>, Reactions>::new(),
                 |mut reactions,
                  Reaction {
                      location,
                      emoji,
                      authors,
                  }| {
                     let mut inner = authors
                         .into_iter()
                         .map(|author| (author, emoji))
                         .collect::<BTreeSet<_>>();
                     let entry = reactions.entry(location).or_default();
                     entry.append(&mut inner);
                     reactions
                 },
             )
         }
     }
     /// Helper to serialize a `Revision`'s reactions, since
     /// `CodeLocation` cannot be a key for a JSON object.
     ///
     /// The set `reactions` are first turned into a set of
     /// [`Reaction`]s and then serialized via a `Vec`.
     pub fn serialize_reactions<S>(
         reactions: &BTreeMap<Option<CodeLocation>, Reactions>,
         serializer: S,
     ) -> Result<S::Ok, S::Error>
     where
         S: serde::Serializer,
     {
         let reactions = reactions
             .iter()
             .flat_map(|(location, reaction)| {
                 let reactions = reaction.iter().fold(
                     BTreeMap::new(),
                     |mut acc: BTreeMap<&super::Reaction, Vec<_>>, (author, emoji)| {
                         acc.entry(emoji).or_default().push(*author);
                         acc
                     },
                 );
                 reactions
                     .into_iter()
                     .map(|(emoji, authors)| Reaction {
                         location: location.clone(),
                         emoji: *emoji,
                         authors,
                     })
                     .collect::<Vec<_>>()
             })
             .collect::<Vec<_>>();
         let mut s = serializer.serialize_seq(Some(reactions.len()))?;
         for r in &reactions {
             s.serialize_element(r)?;
         }
         s.end()
     }
     /// Helper to deserialize a `Revision`'s reactions, the inverse of
     /// `serialize_reactions`.
     ///
     /// The `Vec` of [`Reaction`]s are deserialized and converted to a
     /// `BTreeMap<Option<CodeLocation>, Reactions>`.
     pub fn deserialize_reactions<'de, D>(
         deserializer: D,
     ) -> Result<BTreeMap<Option<CodeLocation>, Reactions>, D::Error>
     where
         D: serde::Deserializer<'de>,
     {
         struct ReactionsVisitor;
         impl<'de> serde::de::Visitor<'de> for ReactionsVisitor {
             type Value = Vec<Reaction>;
             fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
                 formatter.write_str("a reaction of the form {'location', 'emoji', 'authors'}")
             }
             fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
             where
                 A: serde::de::SeqAccess<'de>,
             {
                 let mut reactions = Vec::new();
                 while let Some(reaction) = seq.next_element()? {
                     reactions.push(reaction);
                 }
                 Ok(reactions)
             }
         }
         let reactions = deserializer.deserialize_seq(ReactionsVisitor)?;
         Ok(Reaction::as_revision_reactions(reactions))
     }
 }
 #[cfg(test)]
 #[allow(clippy::unwrap_used)]
 mod test {
     use std::path::PathBuf;
     use std::path::Path;
     use std::str::FromStr;
     use std::vec;

@@ -3006,10 +2811,10 @@ mod test {

         #[serde(rename_all = "camelCase")]
         struct TestReactions {
             #[serde(
                 serialize_with = "super::ser::serialize_reactions",
                 deserialize_with = "super::ser::deserialize_reactions"
                 serialize_with = "encoding::ser::serialize_reactions",
                 deserialize_with = "encoding::ser::deserialize_reactions"
             )]
             inner: BTreeMap<Option<CodeLocation>, Reactions>,
             inner: BTreeMap<Option<encoding::Location>, Reactions>,
         }
         let reactions = TestReactions {

@@ -3370,12 +3175,12 @@ mod test {

                 target: MergeTarget::Delegates,
             },
         ]);
         let a2 = alice.op::<Patch>([Action::RevisionReact {
             revision: RevisionId(a1.id()),
             location: None,
         let a2 = alice.op::<Patch>([Action::from(actions::RevisionReact::new(
             RevisionId(a1.id()),
             None,
             reaction,
             active: true,
         }]);
             true,
         ))]);
         let patch = Patch::from_ops([a1, a2], &repo).unwrap();
         let (_, r1) = patch.revisions().next().unwrap();

@@ -3534,11 +3339,11 @@ mod test {

             .unwrap();
         let (rid, _) = patch.latest();
         let location = CodeLocation {
             commit: branch.oid,
             path: PathBuf::from_str("README").unwrap(),
             old: None,
             new: Some(CodeRange::Lines { range: 5..8 }),
         let location = DiffLocation {
             base: branch.base,
             head: branch.oid,
             path: Path::new("README.md").to_path_buf(),
             selection: None,
         };
         let review = patch
             .review(rid, Some(Verdict::Accept), None, vec![], &alice.signer)

modified crates/radicle/src/cob/patch/actions.rs

@@ -3,11 +3,16 @@

 //!
 //! [`Action`]: super::Action
 use radicle_cob::EntryId;
 use serde::{Deserialize, Serialize};
 use crate::cob::{thread::Edit, ActorId, Embed, Label, Timestamp, Uri};
 use crate::cob::{
     thread,
     thread::{CommentId, Edit},
     ActorId, DiffLocation, Embed, Label, PartialLocation, Reaction, Timestamp, Uri,
 };
 use super::{lookup, Error, Patch, ReviewId, Verdict};
 use super::{encoding::Location, lookup, Error, Patch, ReviewId, RevisionId, Verdict};
 /// A review edit that keeps track of the different versions of actions.
 ///

@@ -178,6 +183,468 @@ pub struct ReviewEditV1 {

     labels: Vec<Label>,
 }
 /// A review comment that keeps track of the different versions of actions.
 ///
 /// [`ReviewComment::new`] will create the latest version of the action.
 ///
 /// [`ReviewComment::run`] will apply the action to the given [`Patch`].
 #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
 #[serde(tag = "type", rename_all = "camelCase")]
 pub enum ReviewComment {
     #[serde(rename = "review.comment")]
     V1(ReviewCommentV1),
     #[serde(rename = "review.comment.v2")]
     V2(ReviewCommentV2),
 }
 impl ReviewComment {
     /// Create the latest version of [`ReviewComment`].
     pub fn new(
         review: ReviewId,
         body: String,
         location: Option<DiffLocation>,
         reply_to: Option<CommentId>,
         embeds: Vec<Embed<Uri>>,
     ) -> Self {
         Self::V2(ReviewCommentV2 {
             review,
             body,
             location,
             reply_to,
             embeds,
         })
     }
     /// Get the [`ReviewId`] that this comment is applying to.
     pub fn review_id(&self) -> &ReviewId {
         match self {
             ReviewComment::V1(v1) => &v1.review,
             ReviewComment::V2(v2) => &v2.review,
         }
     }
     /// Get the body of the comment.
     pub fn body(&self) -> &String {
         match self {
             ReviewComment::V1(v1) => &v1.body,
             ReviewComment::V2(v2) => &v2.body,
         }
     }
     /// Get the [`CommentId`] this comment is replying to, if any.
     pub fn reply_to(&self) -> Option<&CommentId> {
         match self {
             ReviewComment::V1(v1) => v1.reply_to.as_ref(),
             ReviewComment::V2(v2) => v2.reply_to.as_ref(),
         }
     }
     /// Get the [`Embed`]s for this comment.
     pub fn embeds(&self) -> &[Embed<Uri>] {
         match self {
             ReviewComment::V1(v1) => &v1.embeds,
             ReviewComment::V2(v2) => &v2.embeds,
         }
     }
     /// Get the [`DiffLocation`] this comment is referring to, if any.
     pub fn location(&self) -> Option<&DiffLocation> {
         match self {
             ReviewComment::V1(_) => None,
             ReviewComment::V2(v2) => v2.location.as_ref(),
         }
     }
     /// Apply the action to the given [`Patch`].
     pub fn run(
         self,
         entry: EntryId,
         author: ActorId,
         timestamp: Timestamp,
         patch: &mut Patch,
     ) -> Result<(), Error> {
         match self {
             ReviewComment::V1(ReviewCommentV1 {
                 review,
                 body,
                 location,
                 reply_to,
                 embeds,
             }) => {
                 let context = patch
                     .revisions()
                     .find(|(_, rev)| rev.reviews().any(|(_, r)| r.id == review))
                     .map(|(_, rev)| rev.base)
                     // TODO
                     .expect("BUG: no revision for review");
                 if let Some(review) = lookup::review_mut(patch, &review)? {
                     let location =
                         location.map(|loc| Location::V1(loc).into_diff_location(context));
                     thread::comment(
                         &mut review.comments,
                         entry,
                         author,
                         timestamp,
                         body,
                         reply_to,
                         location,
                         embeds,
                     )?;
                 }
             }
             ReviewComment::V2(ReviewCommentV2 {
                 review,
                 body,
                 location,
                 reply_to,
                 embeds,
             }) => {
                 if let Some(review) = lookup::review_mut(patch, &review)? {
                     thread::comment(
                         &mut review.comments,
                         entry,
                         author,
                         timestamp,
                         body,
                         reply_to,
                         location,
                         embeds,
                     )?;
                 }
             }
         }
         Ok(())
     }
 }
 #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct ReviewCommentV2 {
     review: ReviewId,
     body: String,
     #[serde(default, skip_serializing_if = "Option::is_none")]
     location: Option<DiffLocation>,
     /// Comment this is a reply to.
     /// Should be [`None`] if it's the first comment.
     /// Should be [`Some`] otherwise.
     #[serde(default, skip_serializing_if = "Option::is_none")]
     reply_to: Option<CommentId>,
     /// Embeded content.
     #[serde(default, skip_serializing_if = "Vec::is_empty")]
     embeds: Vec<Embed<Uri>>,
 }
 #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct ReviewCommentV1 {
     review: ReviewId,
     body: String,
     #[serde(default, skip_serializing_if = "Option::is_none")]
     location: Option<PartialLocation>,
     /// Comment this is a reply to.
     /// Should be [`None`] if it's the first comment.
     /// Should be [`Some`] otherwise.
     #[serde(default, skip_serializing_if = "Option::is_none")]
     reply_to: Option<CommentId>,
     /// Embeded content.
     #[serde(default, skip_serializing_if = "Vec::is_empty")]
     embeds: Vec<Embed<Uri>>,
 }
 /// A revision comment that keeps track of the different versions of actions.
 ///
 /// [`RevisionComment::new`] will create the latest version of the action.
 ///
 /// [`RevisionComment::run`] will apply the action to the given [`Patch`].
 #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
 #[serde(tag = "type", rename_all = "camelCase")]
 pub enum RevisionComment {
     #[serde(rename = "revision.comment")]
     V1(RevisionCommentV1),
     #[serde(rename = "revision.comment.v2")]
     V2(RevisionCommentV2),
 }
 impl RevisionComment {
     /// Create the latest version of [`RevisionComment`].
     pub fn new(
         revision: RevisionId,
         location: Option<DiffLocation>,
         body: String,
         reply_to: Option<CommentId>,
         embeds: Vec<Embed<Uri>>,
     ) -> Self {
         Self::V2(RevisionCommentV2 {
             revision,
             location,
             body,
             reply_to,
             embeds,
         })
     }
     /// Get the [`RevisionId`] that this comment is applying to.
     pub fn revision(&self) -> &RevisionId {
         match self {
             Self::V1(v1) => &v1.revision,
             Self::V2(v2) => &v2.revision,
         }
     }
     /// Get the body of the comment.
     pub fn body(&self) -> &String {
         match self {
             Self::V1(v1) => &v1.body,
             Self::V2(v2) => &v2.body,
         }
     }
     /// Get the [`CommentId`] this comment is replying to, if any.
     pub fn reply_to(&self) -> Option<&CommentId> {
         match self {
             Self::V1(v1) => v1.reply_to.as_ref(),
             Self::V2(v2) => v2.reply_to.as_ref(),
         }
     }
     /// Get the [`Embed`]s for this comment.
     pub fn embeds(&self) -> &[Embed<Uri>] {
         match self {
             Self::V1(v1) => &v1.embeds,
             Self::V2(v2) => &v2.embeds,
         }
     }
     /// Get the [`DiffLocation`] this comment is referring to, if any.
     pub fn location(&self) -> Option<&DiffLocation> {
         match self {
             Self::V1(_) => None,
             Self::V2(v2) => v2.location.as_ref(),
         }
     }
     /// Apply the action to the given [`Patch`].
     pub fn run(
         self,
         entry: EntryId,
         author: ActorId,
         timestamp: Timestamp,
         patch: &mut Patch,
     ) -> Result<(), Error> {
         match self {
             Self::V1(RevisionCommentV1 {
                 revision,
                 location,
                 body,
                 reply_to,
                 embeds,
             }) => {
                 if let Some(revision) = lookup::revision_mut(patch, &revision)? {
                     let location =
                         location.map(|loc| Location::V1(loc).into_diff_location(revision.base));
                     thread::comment(
                         &mut revision.discussion,
                         entry,
                         author,
                         timestamp,
                         body,
                         reply_to,
                         location,
                         embeds,
                     )?;
                 }
             }
             Self::V2(RevisionCommentV2 {
                 revision,
                 body,
                 location,
                 reply_to,
                 embeds,
             }) => {
                 if let Some(revision) = lookup::revision_mut(patch, &revision)? {
                     thread::comment(
                         &mut revision.discussion,
                         entry,
                         author,
                         timestamp,
                         body,
                         reply_to,
                         location,
                         embeds,
                     )?;
                 }
             }
         }
         Ok(())
     }
 }
 #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct RevisionCommentV2 {
     /// The revision to comment on.
     revision: RevisionId,
     /// For comments on the revision code.
     #[serde(default, skip_serializing_if = "Option::is_none")]
     location: Option<DiffLocation>,
     /// Comment body.
     body: String,
     /// Comment this is a reply to.
     /// Should be [`None`] if it's the top-level comment.
     /// Should be the root [`CommentId`] if it's a top-level comment.
     #[serde(default, skip_serializing_if = "Option::is_none")]
     reply_to: Option<CommentId>,
     /// Embeded content.
     #[serde(default, skip_serializing_if = "Vec::is_empty")]
     embeds: Vec<Embed<Uri>>,
 }
 #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct RevisionCommentV1 {
     /// The revision to comment on.
     revision: RevisionId,
     /// For comments on the revision code.
     #[serde(default, skip_serializing_if = "Option::is_none")]
     location: Option<PartialLocation>,
     /// Comment body.
     body: String,
     /// Comment this is a reply to.
     /// Should be [`None`] if it's the top-level comment.
     /// Should be the root [`CommentId`] if it's a top-level comment.
     #[serde(default, skip_serializing_if = "Option::is_none")]
     reply_to: Option<CommentId>,
     /// Embeded content.
     #[serde(default, skip_serializing_if = "Vec::is_empty")]
     embeds: Vec<Embed<Uri>>,
 }
 /// A revision reaction that keeps track of the different versions of actions.
 ///
 /// [`RevisionReact::new`] will create the latest version of the action.
 ///
 /// [`RevisionReact::run`] will apply the action to the given [`Patch`].
 #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
 #[serde(tag = "type", rename_all = "camelCase")]
 pub enum RevisionReact {
     #[serde(rename = "revision.react")]
     V1(RevisionReactV1),
     #[serde(rename = "revision.react.v2")]
     V2(RevisionReactV2),
 }
 impl RevisionReact {
     /// Create the latest version of [`RevisionReact`].
     pub fn new(
         revision: RevisionId,
         location: Option<DiffLocation>,
         reaction: Reaction,
         active: bool,
     ) -> Self {
         Self::V2(RevisionReactV2 {
             revision,
             location,
             reaction,
             active,
         })
     }
     /// Get the [`RevisionId`] that this reaction is applying to.
     pub fn revision(&self) -> &RevisionId {
         match self {
             Self::V1(v1) => &v1.revision,
             Self::V2(v2) => &v2.revision,
         }
     }
     /// The [`Reaction`] to the revision.
     pub fn reaction(&self) -> &Reaction {
         match self {
             Self::V1(v1) => &v1.reaction,
             Self::V2(v2) => &v2.reaction,
         }
     }
     /// Whether the [`Reaction`] is active.
     pub fn active(&self) -> bool {
         match self {
             Self::V1(v1) => v1.active,
             Self::V2(v2) => v2.active,
         }
     }
     /// Get the [`DiffLocation`] this reaction is referring to, if any.
     pub fn location(&self) -> Option<&DiffLocation> {
         match self {
             Self::V1(_) => None,
             Self::V2(v2) => v2.location.as_ref(),
         }
     }
     /// Apply the action to the given [`Patch`].
     pub fn run(self, author: ActorId, patch: &mut Patch) -> Result<(), Error> {
         match self {
             Self::V1(RevisionReactV1 {
                 revision,
                 location,
                 reaction,
                 active,
             }) => {
                 if let Some(revision) = lookup::revision_mut(patch, &revision)? {
                     let key = (author, reaction);
                     let location =
                         location.map(|loc| Location::V1(loc).into_diff_location(revision.base));
                     let reactions = revision.reactions.entry(location).or_default();
                     if active {
                         reactions.insert(key);
                     } else {
                         reactions.remove(&key);
                     }
                 }
             }
             Self::V2(RevisionReactV2 {
                 revision,
                 location,
                 reaction,
                 active,
             }) => {
                 if let Some(revision) = lookup::revision_mut(patch, &revision)? {
                     let key = (author, reaction);
                     let reactions = revision.reactions.entry(location).or_default();
                     if active {
                         reactions.insert(key);
                     } else {
                         reactions.remove(&key);
                     }
                 }
             }
         }
         Ok(())
     }
 }
 #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct RevisionReactV2 {
     revision: RevisionId,
     #[serde(default, skip_serializing_if = "Option::is_none")]
     location: Option<DiffLocation>,
     reaction: Reaction,
     active: bool,
 }
 #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct RevisionReactV1 {
     revision: RevisionId,
     #[serde(default, skip_serializing_if = "Option::is_none")]
     location: Option<PartialLocation>,
     reaction: Reaction,
     active: bool,
 }
 #[allow(clippy::unwrap_used)]
 #[cfg(test)]
 mod test {

@@ -185,7 +652,7 @@ mod test {

     use crate::patch;
     use super::ReviewEdit;
     use super::*;
     #[test]
     fn test_review_edit() {

@@ -215,4 +682,185 @@ mod test {

             patch::Action::ReviewEdit { .. }
         ));
     }
     #[test]
     fn test_review_comment() {
         let v1 = json!({
             "type": "review.comment",
             "review": "89d45fb371eb2622ba88188d474347cc526d80bb",
             "body": "This looks good to me",
             "location": {
                 "commit": "b455c819807cd7a7543d03215570c72b7cb452d7",
                 "path": "src/main.rs",
                 "old": {
                     "type": "lines",
                     "range": {
                         "start": 10,
                         "end": 12
                     }
                 },
                 "new": {
                     "type": "lines",
                     "range": {
                         "start": 12,
                         "end": 14
                     }
                 }
             },
             "replyTo": "92d77f9ec373261d91a65e68c95baaaa9fc9b95e",
             "embeds": []
         });
         let v2 = json!({
             "type": "review.comment.v2",
             "review": "89d45fb371eb2622ba88188d474347cc526d80bb",
             "body": "This looks good to me",
             "location": {
                 "base": "b455c819807cd7a7543d03215570c72b7cb452d7",
                 "head": "e67a8f4d32c830c24ed68ea21707923480830511",
                 "path": "src/main.rs",
                 "selection": {
                     "hunk": 0,
                     "range": {
                         "type": "lines",
                         "range": {
                             "start": 2,
                             "end": 4
                         }
                     }
                 }
             },
             "replyTo": "92d77f9ec373261d91a65e68c95baaaa9fc9b95e",
             "embeds": []
         });
         serde_json::from_value::<ReviewComment>(v1.clone()).unwrap();
         serde_json::from_value::<ReviewComment>(v2.clone()).unwrap();
         assert!(matches!(
             serde_json::from_value::<patch::Action>(v1).unwrap(),
             patch::Action::ReviewComment { .. }
         ));
         assert!(matches!(
             serde_json::from_value::<patch::Action>(v2).unwrap(),
             patch::Action::ReviewComment { .. }
         ));
     }
     #[test]
     fn test_revision_comment() {
         let v1 = json!({
             "type": "revision.comment",
             "revision": "b455c819807cd7a7543d03215570c72b7cb452d7",
             "body": "Nice changes here",
             "location": {
                 "commit": "e67a8f4d32c830c24ed68ea21707923480830511",
                 "path": "src/lib.rs",
                 "old": {
                     "type": "lines",
                     "range": {
                         "start": 25,
                         "end": 27
                     }
                 },
                 "new": {
                     "type": "lines",
                     "range": {
                         "start": 30,
                         "end": 32
                     }
                 }
             },
             "replyTo": "05368e84fabb0fa5d2995741de87374812ae501c",
             "embeds": []
         });
         let v2 = json!({
             "type": "revision.comment.v2",
             "revision": "b455c819807cd7a7543d03215570c72b7cb452d7",
             "body": "Nice changes here",
             "location": {
                 "base": "e67a8f4d32c830c24ed68ea21707923480830511",
                 "head": "2a47bc0c7dd6238ce3416e19d2ba57f3a7626f64",
                 "path": "src/lib.rs",
                 "selection": {
                     "hunk": 1,
                     "range": {
                         "type": "lines",
                         "range": {
                             "start": 0,
                             "end": 3
                         }
                     }
                 }
             },
             "replyTo": "05368e84fabb0fa5d2995741de87374812ae501c",
             "embeds": []
         });
         serde_json::from_value::<RevisionComment>(v1.clone()).unwrap();
         serde_json::from_value::<RevisionComment>(v2.clone()).unwrap();
         assert!(matches!(
             serde_json::from_value::<patch::Action>(v1).unwrap(),
             patch::Action::RevisionComment { .. }
         ));
         assert!(matches!(
             serde_json::from_value::<patch::Action>(v2).unwrap(),
             patch::Action::RevisionComment { .. }
         ));
     }
     #[test]
     fn test_revision_react() {
         let v1 = json!({
             "type": "revision.react",
             "revision": "e67a8f4d32c830c24ed68ea21707923480830511",
             "reaction": "👍",
             "active": true,
             "location": {
                 "commit": "2a47bc0c7dd6238ce3416e19d2ba57f3a7626f64",
                 "path": "src/utils.rs",
                 "old": {
                     "type": "lines",
                     "range": {
                         "start": 15,
                         "end": 17
                     }
                 },
                 "new": {
                     "type": "lines",
                     "range": {
                         "start": 18,
                         "end": 20
                     }
                 }
             }
         });
         let v2 = json!({
             "type": "revision.react.v2",
             "revision": "e67a8f4d32c830c24ed68ea21707923480830511",
             "reaction": "👍",
             "active": true,
             "location": {
                 "base": "2a47bc0c7dd6238ce3416e19d2ba57f3a7626f64",
                 "head": "a998ce691d6962ea75d861b25532f4c042c36f1a",
                 "path": "src/utils.rs",
                 "selection": {
                     "hunk": 2,
                     "range": {
                         "type": "lines",
                         "range": {
                             "start": 5,
                             "end": 8
                         }
                     }
                 }
             }
         });
         serde_json::from_value::<RevisionReact>(v1.clone()).unwrap();
         serde_json::from_value::<RevisionReact>(v2.clone()).unwrap();
         assert!(matches!(
             serde_json::from_value::<patch::Action>(v1).unwrap(),
             patch::Action::RevisionReact { .. }
         ));
         assert!(matches!(
             serde_json::from_value::<patch::Action>(v2).unwrap(),
             patch::Action::RevisionReact { .. }
         ));
     }
 }

modified crates/radicle/src/cob/patch/encoding.rs

@@ -4,3 +4,163 @@

 //! [`Patch`]: super::Patch
 pub mod review;
 pub mod revision;
 use crate::cob::{DiffLocation, PartialLocation};
 use crate::git;
 #[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, serde::Serialize, serde::Deserialize)]
 #[serde(untagged)]
 pub(in crate::cob::patch) enum Location {
     V2(DiffLocation),
     V1(PartialLocation),
 }
 impl Location {
     pub fn into_diff_location(self, context: git::Oid) -> DiffLocation {
         match self {
             Location::V2(loc) => loc,
             Location::V1(PartialLocation { commit, path, .. }) => DiffLocation {
                 base: context,
                 head: commit,
                 path,
                 // N.b. We would need to figure out where the lines translate it
                 // into the hunk – which would require using a `Repository` to
                 // look up.
                 // Instead, we just set the selection to `None` and preserve the
                 // rest of the information.
                 selection: None,
             },
         }
     }
 }
 /// Helpers for de/serialization of patch data types.
 pub(super) mod ser {
     use std::collections::BTreeMap;
     use serde::{ser::SerializeSeq as _, Deserialize, Serialize};
     use crate::cob::{patch, thread::Reactions, ActorId};
     #[cfg(test)]
     use std::{collections::BTreeSet, marker::PhantomData};
     /// Serialize a `Revision`'s reaction as an object containing the
     /// `location`, `emoji`, and all `authors` that have performed the
     /// same reaction.
     #[derive(Debug, Serialize, Deserialize)]
     #[serde(rename_all = "camelCase")]
     struct Reaction<T> {
         location: Option<T>,
         emoji: patch::Reaction,
         authors: Vec<ActorId>,
     }
     impl<T> Reaction<T> {
         #[cfg(test)]
         fn as_revision_reactions(reactions: Vec<Self>) -> BTreeMap<Option<T>, Reactions>
         where
             T: Ord,
         {
             reactions.into_iter().fold(
                 BTreeMap::<Option<T>, Reactions>::new(),
                 |mut reactions,
                  Reaction {
                      location,
                      emoji,
                      authors,
                  }| {
                     let mut inner = authors
                         .into_iter()
                         .map(|author| (author, emoji))
                         .collect::<BTreeSet<_>>();
                     let entry = reactions.entry(location).or_default();
                     entry.append(&mut inner);
                     reactions
                 },
             )
         }
     }
     /// Helper to serialize a `Revision`'s reactions, since
     /// `CodeLocation` cannot be a key for a JSON object.
     ///
     /// The set `reactions` are first turned into a set of
     /// [`Reaction`]s and then serialized via a `Vec`.
     pub fn serialize_reactions<T, S>(
         reactions: &BTreeMap<Option<T>, Reactions>,
         serializer: S,
     ) -> Result<S::Ok, S::Error>
     where
         S: serde::Serializer,
         T: Clone + Serialize,
     {
         let reactions = reactions
             .iter()
             .flat_map(|(location, reaction)| {
                 let reactions = reaction.iter().fold(
                     BTreeMap::new(),
                     |mut acc: BTreeMap<&patch::Reaction, Vec<_>>, (author, emoji)| {
                         acc.entry(emoji).or_default().push(*author);
                         acc
                     },
                 );
                 reactions
                     .into_iter()
                     .map(|(emoji, authors)| Reaction {
                         location: location.clone(),
                         emoji: *emoji,
                         authors,
                     })
                     .collect::<Vec<_>>()
             })
             .collect::<Vec<_>>();
         let mut s = serializer.serialize_seq(Some(reactions.len()))?;
         for r in &reactions {
             s.serialize_element(r)?;
         }
         s.end()
     }
     /// Helper to deserialize a `Revision`'s reactions, the inverse of
     /// `serialize_reactions`.
     ///
     /// The `Vec` of [`Reaction`]s are deserialized and converted to a
     /// `BTreeMap<Option<CodeLocation>, Reactions>`.
     #[cfg(test)]
     pub fn deserialize_reactions<'de, T, D>(
         deserializer: D,
     ) -> Result<BTreeMap<Option<T>, Reactions>, D::Error>
     where
         D: serde::Deserializer<'de>,
         T: Ord + Deserialize<'de>,
     {
         struct ReactionsVisitor<T>(PhantomData<T>);
         impl<'de, T> serde::de::Visitor<'de> for ReactionsVisitor<T>
         where
             T: Deserialize<'de>,
         {
             type Value = Vec<Reaction<T>>;
             fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
                 formatter.write_str("a reaction of the form {'location', 'emoji', 'authors'}")
             }
             fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
             where
                 A: serde::de::SeqAccess<'de>,
             {
                 let mut reactions = Vec::new();
                 while let Some(reaction) = seq.next_element()? {
                     reactions.push(reaction);
                 }
                 Ok(reactions)
             }
         }
         let reactions = deserializer.deserialize_seq(ReactionsVisitor::<T>(PhantomData))?;
         Ok(Reaction::as_revision_reactions(reactions))
     }
 }

modified crates/radicle/src/cob/patch/encoding/review.rs

@@ -14,8 +14,11 @@ use serde_untagged::UntaggedEnumVisitor;

 use crate::cob::patch;
 use crate::cob::patch::{
     Author, CodeLocation, Comment, Edit, Label, Reactions, ReviewId, Thread, Timestamp, Verdict,
     Author, Comment, Edit, Label, Reactions, ReviewId, Thread, Timestamp, Verdict,
 };
 use crate::git;
 use super::Location;
 /// The encoding for a `patch::Review` that can be deserialized and migrated.
 ///

@@ -29,7 +32,6 @@ pub(in crate::cob::patch) struct Review {

     id: ReviewId,
     author: Author,
     verdict: Option<Verdict>,
     comments: Thread<Comment<CodeLocation>>,
     labels: Vec<Label>,
     timestamp: Timestamp,

@@ -40,6 +42,40 @@ pub(in crate::cob::patch) struct Review {

     // V1 -> V2 conversion
     #[serde(default)]
     summary: Summary,
     comments: Thread<Comment<Location>>,
 }
 impl Review {
     pub fn decode(self, context: git::Oid) -> patch::Review {
         let Review {
             id,
             author,
             verdict,
             comments: Thread { comments, timeline },
             labels,
             timestamp,
             reactions,
             summary,
         } = self;
         let summary = summary.into_edits(&author, &timestamp);
         let comments = comments
             .into_iter()
             .map(|(id, comment)| {
                 let comment = comment.map(|c| c.map(|l| l.into_diff_location(context)));
                 (id, comment)
             })
             .collect();
         patch::Review {
             id,
             author,
             verdict,
             summary,
             comments: Thread { comments, timeline },
             labels,
             reactions,
             timestamp,
         }
     }
 }
 /// The [`Summary`] type represents the different versions of the `summary`

@@ -94,45 +130,44 @@ impl Summary {

     }
 }
 impl From<Review> for patch::Review {
     fn from(review: Review) -> Self {
         let Review {
             id,
             author,
             verdict,
             comments,
             labels,
             timestamp,
             reactions,
             summary,
         } = review;
         let summary = summary.into_edits(&author, &timestamp);
         Self {
             id,
             author,
             verdict,
             summary,
             comments,
             labels,
             reactions,
             timestamp,
         }
     }
 }
 #[allow(clippy::unwrap_used)]
 #[cfg(test)]
 mod test {
     use nonempty::nonempty;
     use std::collections::BTreeSet;
     use nonempty::{nonempty, NonEmpty};
     use serde_json::json;
     use crate::{
         cob::{thread::Edit, Timestamp},
         patch,
         cob::{
             thread::{Edit, Thread},
             Author, Timestamp,
         },
         patch::{self, ReviewId, Verdict},
         prelude::Did,
     };
     use super::{Review, Summary};
     fn author() -> Author {
         Author::new(
             "did:key:z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx"
                 .parse::<Did>()
                 .unwrap(),
         )
     }
     fn timestamp() -> Timestamp {
         Timestamp::from_secs(1710947885_u64)
     }
     fn review_id() -> ReviewId {
         "89d45fb371eb2622ba88188d474347cc526d80bb"
             .parse::<crate::cob::EntryId>()
             .unwrap()
             .into()
     }
     #[test]
     fn test_review_summary() {
         let summary_null = json!(null);

@@ -179,9 +214,25 @@ mod test {

             "timestamp": 1710947885000_i64
         });
         let v1 = serde_json::from_value::<Review>(review.clone()).unwrap();
         let author = author();
         let timestamp = timestamp();
         assert_eq!(
             serde_json::from_value::<patch::Review>(review).unwrap(),
             v1.into()
             v1.decode(git2::Oid::zero().into()),
             patch::Review {
                 id: review_id(),
                 author: author.clone(),
                 verdict: Some(Verdict::Accept),
                 summary: NonEmpty::new(Edit::new(
                     *author.public_key(),
                     "".to_string(),
                     timestamp,
                     vec![]
                 )),
                 comments: Thread::default(),
                 labels: vec![],
                 reactions: BTreeSet::new(),
                 timestamp,
             }
         );
     }

@@ -199,9 +250,25 @@ mod test {

             "timestamp": 1710947885000_i64
         });
         let v1 = serde_json::from_value::<Review>(review.clone()).unwrap();
         let author = author();
         let timestamp = timestamp();
         assert_eq!(
             serde_json::from_value::<patch::Review>(review).unwrap(),
             v1.into()
             v1.decode(git2::Oid::zero().into()),
             patch::Review {
                 id: review_id(),
                 author: author.clone(),
                 verdict: Some(Verdict::Accept),
                 summary: NonEmpty::new(Edit::new(
                     *author.public_key(),
                     "".to_string(),
                     timestamp,
                     vec![]
                 )),
                 comments: Thread::default(),
                 labels: vec![],
                 reactions: BTreeSet::new(),
                 timestamp,
             }
         );
     }

@@ -219,10 +286,27 @@ mod test {

             "labels": [],
             "timestamp": 1710947885000_i64
         });
         let v1 = serde_json::from_value::<Review>(review.clone()).unwrap();
         let author = author();
         let timestamp = timestamp();
         assert_eq!(
             serde_json::from_value::<patch::Review>(review).unwrap(),
             v1.into()
             v1.decode(git2::Oid::zero().into()),
             patch::Review {
                 id: review_id(),
                 author: author.clone(),
                 verdict: Some(Verdict::Accept),
                 summary: NonEmpty::new(Edit::new(
                     *author.public_key(),
                     "lgtm".to_string(),
                     timestamp,
                     vec![]
                 )),
                 comments: Thread::default(),
                 labels: vec![],
                 reactions: BTreeSet::new(),
                 timestamp,
             }
         );
     }

@@ -246,9 +330,25 @@ mod test {

             "timestamp": 1710947885000_i64
         });
         let v2 = serde_json::from_value::<Review>(review.clone()).unwrap();
         let author = author();
         let timestamp = timestamp();
         assert_eq!(
             serde_json::from_value::<patch::Review>(review).unwrap(),
             v2.into()
             v2.decode(git2::Oid::zero().into()),
             patch::Review {
                 id: review_id(),
                 author: author.clone(),
                 verdict: Some(Verdict::Accept),
                 summary: NonEmpty::new(Edit::new(
                     *author.public_key(),
                     "lgtm".to_string(),
                     timestamp,
                     vec![]
                 )),
                 comments: Thread::default(),
                 labels: vec![],
                 reactions: BTreeSet::new(),
                 timestamp,
             }
         );
     }
 }

added crates/radicle/src/cob/patch/encoding/revision.rs

@@ -0,0 +1,606 @@

 use std::collections::{BTreeMap, BTreeSet};
 use nonempty::NonEmpty;
 use radicle_cob::EntryId;
 use serde::Deserialize;
 use crate::{
     cob::{
         thread::{Comment, CommentId, Edit, Reactions, Thread},
         ActorId, Author, DiffLocation, Timestamp,
     },
     git,
     patch::{self, encoding, RevisionId},
 };
 use super::Location;
 /// A patch revision.
 #[derive(Deserialize)]
 #[serde(rename_all = "camelCase")]
 pub struct Revision {
     /// Revision identifier.
     id: RevisionId,
     /// Author of the revision.
     author: Author,
     /// Revision description.
     description: NonEmpty<Edit>,
     /// Base branch commit, used as a merge base.
     base: git::Oid,
     /// Reference to the Git object containing the code (revision head).
     oid: git::Oid,
     /// When this revision was created.
     timestamp: Timestamp,
     /// Review comments resolved by this revision.
     resolves: BTreeSet<(EntryId, CommentId)>,
     // V1 -> V2 conversion
     /// Discussion around this revision.
     discussion: Thread<Comment<Location>>,
     reviews: BTreeMap<ActorId, encoding::review::Review>,
     /// Reactions on code locations and revision itself
     #[serde(deserialize_with = "ser::deserialize_reactions")]
     reactions: BTreeMap<Option<Location>, Reactions>,
 }
 impl From<Revision> for patch::Revision {
     fn from(revision: Revision) -> Self {
         let Revision {
             id,
             author,
             description,
             base,
             oid,
             timestamp,
             resolves,
             discussion,
             reviews,
             reactions,
         } = revision;
         let discussion = decode_discussion(discussion, base);
         let reviews = decode_reviews(reviews, base);
         let reactions = decode_reactions(reactions, base);
         Self {
             id,
             author,
             description,
             base,
             oid,
             discussion,
             reviews,
             timestamp,
             resolves,
             reactions,
         }
     }
 }
 fn decode_reviews(
     reviews: BTreeMap<ActorId, encoding::review::Review>,
     context: git::Oid,
 ) -> BTreeMap<ActorId, patch::Review> {
     reviews
         .into_iter()
         .map(|(actor, review)| (actor, review.decode(context)))
         .collect()
 }
 fn decode_discussion(
     discussion: Thread<Comment<Location>>,
     context: git::Oid,
 ) -> Thread<Comment<DiffLocation>> {
     let comments = discussion
         .comments
         .into_iter()
         .map(|(id, c)| {
             let c = c.map(|c| c.map(|loc| loc.into_diff_location(context)));
             (id, c)
         })
         .collect();
     Thread {
         comments,
         timeline: discussion.timeline,
     }
 }
 fn decode_reactions(
     reactions: BTreeMap<Option<Location>, Reactions>,
     context: git::Oid,
 ) -> BTreeMap<Option<DiffLocation>, Reactions> {
     reactions
         .into_iter()
         .map(|(loc, rs)| {
             let loc = loc.map(|loc| loc.into_diff_location(context));
             (loc, rs)
         })
         .collect()
 }
 /// Helpers for de/serialization of patch data types.
 mod ser {
     use std::collections::{BTreeMap, BTreeSet};
     use crate::{
         cob::{patch, thread::Reactions, ActorId},
         patch::encoding::Location,
     };
     /// Serialize a `Revision`'s reaction as an object containing the
     /// `location`, `emoji`, and all `authors` that have performed the
     /// same reaction.
     #[derive(Debug, serde::Deserialize)]
     #[serde(rename_all = "camelCase")]
     struct Reaction {
         location: Option<Location>,
         emoji: patch::Reaction,
         authors: Vec<ActorId>,
     }
     impl Reaction {
         fn as_revision_reactions(
             reactions: Vec<Reaction>,
         ) -> BTreeMap<Option<Location>, Reactions> {
             reactions.into_iter().fold(
                 BTreeMap::<Option<Location>, Reactions>::new(),
                 |mut reactions,
                  Reaction {
                      location,
                      emoji,
                      authors,
                  }| {
                     let mut inner = authors
                         .into_iter()
                         .map(|author| (author, emoji))
                         .collect::<BTreeSet<_>>();
                     let entry = reactions.entry(location).or_default();
                     entry.append(&mut inner);
                     reactions
                 },
             )
         }
     }
     /// Helper to deserialize a `Revision`'s reactions, the inverse of
     /// `serialize_reactions`.
     ///
     /// The `Vec` of [`Reaction`]s are deserialized and converted to a
     /// `BTreeMap<Option<CodeLocation>, Reactions>`.
     pub fn deserialize_reactions<'de, D>(
         deserializer: D,
     ) -> Result<BTreeMap<Option<Location>, Reactions>, D::Error>
     where
         D: serde::Deserializer<'de>,
     {
         struct ReactionsVisitor;
         impl<'de> serde::de::Visitor<'de> for ReactionsVisitor {
             type Value = Vec<Reaction>;
             fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
                 formatter.write_str("a reaction of the form {'location', 'emoji', 'authors'}")
             }
             fn visit_seq<A>(self, mut seq: A) -> Result<Self::Value, A::Error>
             where
                 A: serde::de::SeqAccess<'de>,
             {
                 let mut reactions = Vec::new();
                 while let Some(reaction) = seq.next_element()? {
                     reactions.push(reaction);
                 }
                 Ok(reactions)
             }
         }
         let reactions = deserializer.deserialize_seq(ReactionsVisitor)?;
         Ok(Reaction::as_revision_reactions(reactions))
     }
 }
 #[allow(clippy::unwrap_used)]
 #[cfg(test)]
 mod test {
     use serde_json::json;
     use crate::{
         cob::{Author, DiffLocation, PartialLocation, Reaction},
         git, patch,
         prelude::Did,
     };
     use super::{Location, Revision};
     fn author() -> Author {
         Author::new(
             "did:key:z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx"
                 .parse::<Did>()
                 .unwrap(),
         )
     }
     fn base_oid() -> git::Oid {
         "e67a8f4d32c830c24ed68ea21707923480830511".parse().unwrap()
     }
     fn head_oid() -> git::Oid {
         "b455c819807cd7a7543d03215570c72b7cb452d7".parse().unwrap()
     }
     #[test]
     fn test_revision_deserialize_v1_location_migration() {
         let revision_json = json!({
             "id": "89d45fb371eb2622ba88188d474347cc526d80bb",
             "author": { "id": "did:key:z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx" },
             "description": [{
                 "author": "z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx",
                 "timestamp": 1710947885000_i64,
                 "body": "Initial revision",
                 "embeds": []
             }],
             "base": "e67a8f4d32c830c24ed68ea21707923480830511",
             "oid": "b455c819807cd7a7543d03215570c72b7cb452d7",
             "timestamp": 1710947885000_i64,
             "resolves": [],
             "discussion": {
                 "comments": {
                     "2a47bc0c7dd6238ce3416e19d2ba57f3a7626f64": {
                         "id": "2a47bc0c7dd6238ce3416e19d2ba57f3a7626f64",
                         "author": "z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx",
                         "edits": [{
                             "author": "z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx",
                             "timestamp": 1710947885000_i64,
                             "body": "Great changes!",
                             "embeds": []
                         }],
                         "reactions": [],
                         "replyTo": null,
                         "resolved": false,
                         "location": {
                             "commit": "b455c819807cd7a7543d03215570c72b7cb452d7",
                             "path": "src/main.rs",
                             "old": {
                                 "type": "lines",
                                 "range": {
                                     "start": 10,
                                     "end": 12
                                 }
                             },
                             "new": {
                                 "type": "lines",
                                 "range": {
                                     "start": 12,
                                     "end": 14
                                 }
                             }
                         },
                         "embeds": []
                     }
                 },
                 "timeline": ["2a47bc0c7dd6238ce3416e19d2ba57f3a7626f64"]
             },
             "reviews": {},
             "reactions": [{
                 "location": {
                     "commit": "b455c819807cd7a7543d03215570c72b7cb452d7",
                     "path": "src/utils.rs",
                     "old": {
                         "type": "lines",
                         "range": {
                             "start": 5,
                             "end": 7
                         }
                     },
                     "new": {
                         "type": "lines",
                         "range": {
                             "start": 8,
                             "end": 10
                         }
                     }
                 },
                 "emoji": "👍",
                 "authors": ["z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx"]
             }]
         });
         let revision = serde_json::from_value::<Revision>(revision_json).unwrap();
         let decoded: patch::Revision = revision.into();
         // Check that the V1 PartialLocation was migrated to V2 DiffLocation
         let (comment_id, comment) = decoded.discussion().comments().next().unwrap();
         assert_eq!(
             comment_id.to_string(),
             "2a47bc0c7dd6238ce3416e19d2ba57f3a7626f64"
         );
         assert_eq!(comment.body(), "Great changes!");
         let location = comment.location().unwrap();
         assert_eq!(location.base, base_oid());
         assert_eq!(location.head, head_oid());
         assert_eq!(location.path.to_str().unwrap(), "src/main.rs");
         assert!(location.selection.is_none()); // V1 locations lose selection info
         // Check that reactions were also migrated
         let reactions = decoded.reactions();
         assert_eq!(reactions.len(), 1);
         let (location, reaction_set) = reactions.iter().next().unwrap();
         let location = location.as_ref().unwrap();
         assert_eq!(location.base, base_oid());
         assert_eq!(location.head, head_oid());
         assert_eq!(location.path.to_str().unwrap(), "src/utils.rs");
         assert!(location.selection.is_none()); // V1 locations lose selection info
         let reaction_emoji = Reaction::new('👍').unwrap();
         assert!(reaction_set.contains(&(*author().public_key(), reaction_emoji)));
     }
     #[test]
     fn test_revision_deserialize_v2_location_preserved() {
         let revision_json = json!({
             "id": "89d45fb371eb2622ba88188d474347cc526d80bb",
             "author": { "id": "did:key:z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx" },
             "description": [{
                 "author": "z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx",
                 "timestamp": 1710947885000_i64,
                 "body": "Initial revision",
                 "embeds": []
             }],
             "base": "e67a8f4d32c830c24ed68ea21707923480830511",
             "oid": "b455c819807cd7a7543d03215570c72b7cb452d7",
             "timestamp": 1710947885000_i64,
             "resolves": [],
             "discussion": {
                 "comments": {
                     "2a47bc0c7dd6238ce3416e19d2ba57f3a7626f64": {
                         "id": "2a47bc0c7dd6238ce3416e19d2ba57f3a7626f64",
                         "author": "z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx",
                         "edits": [{
                             "author": "z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx",
                             "timestamp": 1710947885000_i64,
                             "body": "Great changes!",
                             "embeds": []
                         }],
                         "reactions": [],
                         "replyTo": null,
                         "resolved": false,
                         "location": {
                             "base": "e67a8f4d32c830c24ed68ea21707923480830511",
                             "head": "b455c819807cd7a7543d03215570c72b7cb452d7",
                             "path": "src/main.rs",
                             "selection": {
                                 "hunk": 0,
                                 "range": {
                                     "type": "lines",
                                     "range": {
                                         "start": 2,
                                         "end": 4
                                     }
                                 }
                             }
                         },
                         "embeds": []
                     }
                 },
                 "timeline": ["2a47bc0c7dd6238ce3416e19d2ba57f3a7626f64"]
             },
             "reviews": {},
             "reactions": [{
                 "location": {
                     "base": "e67a8f4d32c830c24ed68ea21707923480830511",
                     "head": "b455c819807cd7a7543d03215570c72b7cb452d7",
                     "path": "src/utils.rs",
                     "selection": {
                         "hunk": 1,
                         "range": {
                             "type": "lines",
                             "range": {
                                 "start": 0,
                                 "end": 3
                             }
                         }
                     }
                 },
                 "emoji": "👍",
                 "authors": ["z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx"]
             }]
         });
         let revision = serde_json::from_value::<Revision>(revision_json).unwrap();
         let decoded: patch::Revision = revision.into();
         // Check that the V2 DiffLocation was preserved
         let (comment_id, comment) = decoded.discussion().comments().next().unwrap();
         assert_eq!(
             comment_id.to_string(),
             "2a47bc0c7dd6238ce3416e19d2ba57f3a7626f64"
         );
         assert_eq!(comment.body(), "Great changes!");
         let location = comment.location().unwrap();
         assert_eq!(location.base, base_oid());
         assert_eq!(location.head, head_oid());
         assert_eq!(location.path.to_str().unwrap(), "src/main.rs");
         // V2 locations preserve selection info
         let selection = location.selection.as_ref().unwrap();
         assert_eq!(selection.index(), 0);
         assert_eq!(selection.range().line_start(), 2);
         assert_eq!(selection.range().line_end(), 4);
         // Check that reactions were also preserved
         let reactions = decoded.reactions();
         assert_eq!(reactions.len(), 1);
         let (location, reaction_set) = reactions.iter().next().unwrap();
         let location = location.as_ref().unwrap();
         assert_eq!(location.base, base_oid());
         assert_eq!(location.head, head_oid());
         assert_eq!(location.path.to_str().unwrap(), "src/utils.rs");
         // V2 locations preserve selection info
         let selection = location.selection.as_ref().unwrap();
         assert_eq!(selection.index(), 1);
         assert_eq!(selection.range().line_start(), 0);
         assert_eq!(selection.range().line_end(), 3);
         let reaction_emoji = Reaction::new('👍').unwrap();
         assert!(reaction_set.contains(&(*author().public_key(), reaction_emoji)));
     }
     #[test]
     fn test_revision_deserialize_mixed_locations() {
         // Test a revision that has both V1 and V2 locations mixed together
         let revision_json = json!({
             "id": "89d45fb371eb2622ba88188d474347cc526d80bb",
             "author": { "id": "did:key:z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx" },
             "description": [{
                 "author": "z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx",
                 "timestamp": 1710947885000_i64,
                 "body": "Initial revision",
                 "embeds": []
             }],
             "base": "e67a8f4d32c830c24ed68ea21707923480830511",
             "oid": "b455c819807cd7a7543d03215570c72b7cb452d7",
             "timestamp": 1710947885000_i64,
             "resolves": [],
             "discussion": {
                 "comments": {
                     "2a47bc0c7dd6238ce3416e19d2ba57f3a7626f64": {
                         "id": "2a47bc0c7dd6238ce3416e19d2ba57f3a7626f64",
                         "author": "z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx",
                         "edits": [{
                             "author": "z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx",
                             "timestamp": 1710947885000_i64,
                             "body": "V1 location comment",
                             "embeds": []
                         }],
                         "timestamp": 1710947885000_i64,
                         "replyTo": null,
                         "resolved": false,
                         "location": {
                             "commit": "b455c819807cd7a7543d03215570c72b7cb452d7",
                             "path": "src/old.rs",
                             "old": {
                                 "type": "lines",
                                 "range": { "start": 1, "end": 3 }
                             },
                             "new": {
                                 "type": "lines",
                                 "range": { "start": 1, "end": 3 }
                             }
                         },
                         "reactions": [],
                         "embeds": []
                     },
                     "89d45fb371eb2622ba88188d474347cc526d80bb": {
                         "id": "89d45fb371eb2622ba88188d474347cc526d80bb",
                         "author": "z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx",
                         "edits": [{
                             "author": "z6MkwPUeUS2fJMfc2HZN1RQTQcTTuhw4HhPySB8JeUg2mVvx",
                             "timestamp": 1710947885000_i64,
                             "body": "V2 location comment",
                             "embeds": []
                         }],
                         "timestamp": 1710947885000_i64,
                         "replyTo": null,
                         "resolved": false,
                         "location": {
                             "base": "e67a8f4d32c830c24ed68ea21707923480830511",
                             "head": "b455c819807cd7a7543d03215570c72b7cb452d7",
                             "path": "src/new.rs",
                             "selection": {
                                 "hunk": 2,
                                 "range": {
                                     "type": "lines",
                                     "range": { "start": 5, "end": 8 }
                                 }
                             }
                         },
                         "reactions": [],
                         "embeds": []
                     }
                 },
                 "timeline": ["2a47bc0c7dd6238ce3416e19d2ba57f3a7626f64", "89d45fb371eb2622ba88188d474347cc526d80bb"]
             },
             "reviews": {},
             "reactions": []
         });
         let revision = serde_json::from_value::<Revision>(revision_json).unwrap();
         let decoded: patch::Revision = revision.into();
         let comments: Vec<_> = decoded.discussion().comments().collect();
         assert_eq!(comments.len(), 2);
         // Check V1 location was migrated
         let (comment1_id, comment1) = &comments[0];
         assert_eq!(
             comment1_id.to_string(),
             "2a47bc0c7dd6238ce3416e19d2ba57f3a7626f64"
         );
         assert_eq!(comment1.body(), "V1 location comment");
         let location1 = comment1.location().unwrap();
         assert_eq!(location1.base, base_oid()); // Context provided
         assert_eq!(location1.head, head_oid());
         assert_eq!(location1.path.to_str().unwrap(), "src/old.rs");
         assert!(location1.selection.is_none()); // V1 loses selection
         // Check V2 location was preserved
         let (comment2_id, comment2) = &comments[1];
         assert_eq!(
             comment2_id.to_string(),
             "89d45fb371eb2622ba88188d474347cc526d80bb"
         );
         assert_eq!(comment2.body(), "V2 location comment");
         let location2 = comment2.location().unwrap();
         assert_eq!(location2.base, base_oid());
         assert_eq!(location2.head, head_oid());
         assert_eq!(location2.path.to_str().unwrap(), "src/new.rs");
         // V2 preserves selection
         let selection2 = location2.selection.as_ref().unwrap();
         assert_eq!(selection2.index(), 2);
         assert_eq!(selection2.range().line_start(), 5);
         assert_eq!(selection2.range().line_end(), 8);
     }
     #[test]
     fn test_location_v1_to_v2_migration() {
         let base_context = base_oid();
         let partial_location = PartialLocation {
             commit: head_oid(),
             path: "src/test.rs".into(),
             old: Some(crate::cob::common::CodeRange::lines(10..15)),
             new: Some(crate::cob::common::CodeRange::lines(12..17)),
         };
         let v1_location = Location::V1(partial_location);
         let diff_location = v1_location.into_diff_location(base_context);
         assert_eq!(diff_location.base, base_context);
         assert_eq!(diff_location.head, head_oid());
         assert_eq!(diff_location.path.to_str().unwrap(), "src/test.rs");
         assert!(diff_location.selection.is_none()); // V1 migration loses selection
     }
     #[test]
     fn test_location_v2_preserved() {
         let base_context = base_oid();
         let diff_location = DiffLocation {
             base: base_oid(),
             head: head_oid(),
             path: "src/test.rs".into(),
             selection: Some(crate::cob::common::HunkIndex::new(
 ,
                 crate::cob::common::CodeRange::lines(5..10),
             )),
         };
         let v2_location = Location::V2(diff_location.clone());
         let preserved_location = v2_location.into_diff_location(base_context);
         assert_eq!(preserved_location, diff_location); // Should be identical
     }
 }

modified crates/radicle/src/cob/thread.rs

@@ -234,6 +234,21 @@ impl<L> Comment<L> {

     pub fn unresolve(&mut self) {
         self.resolved = false;
     }
     pub(crate) fn map<M, F>(self, f: F) -> Comment<M>
     where
         F: FnOnce(L) -> M,
     {
         let location = self.location.map(f);
         Comment {
             author: self.author,
             edits: self.edits,
             reactions: self.reactions,
             reply_to: self.reply_to,
             location,
             resolved: self.resolved,
         }
     }
 }
 impl<T: PartialOrd> PartialOrd for Comment<T> {