From a61be1b4c4b736aca69fe3ba1b457718ea2ae577 Mon Sep 17 00:00:00 2001 From: Origami404 Date: Wed, 8 Apr 2026 23:20:41 +0800 Subject: [PATCH] fix: avoid merging unrelated comments into replies --- src/gitea/dto.rs | 2 + src/normalize.rs | 78 ++++++++++++++++++--------------------- tests/normalize_tests.rs | 79 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 117 insertions(+), 42 deletions(-) diff --git a/src/gitea/dto.rs b/src/gitea/dto.rs index b9839f2..d8c6a78 100644 --- a/src/gitea/dto.rs +++ b/src/gitea/dto.rs @@ -46,6 +46,8 @@ pub struct ReviewCommentDto { pub path: Option, pub line: Option, pub pull_request_review_id: Option, + #[serde(default, alias = "in_reply_to", alias = "in_reply_to_id", alias = "reply_to")] + pub in_reply_to: Option, pub original_position: Option, pub position: Option, pub commit_id: Option, diff --git a/src/normalize.rs b/src/normalize.rs index 57d0e28..e50c0a6 100644 --- a/src/normalize.rs +++ b/src/normalize.rs @@ -48,59 +48,53 @@ pub fn normalize_bundle(repo: &str, bundle: PullBundleDto) -> PrReviewDocument { } fn normalize_threads(comments: Vec) -> Vec { - let mut grouped: BTreeMap> = BTreeMap::new(); + let mut all = comments; + all.sort_by_key(comment_sort_key); - for comment in comments { - let key = thread_key(&comment); - grouped.entry(key).or_default().push(comment); + let comment_ids: std::collections::BTreeSet = all.iter().map(|comment| comment.id).collect(); + let mut roots: Vec = Vec::new(); + let mut children: BTreeMap> = BTreeMap::new(); + + for comment in all { + if let Some(parent_id) = comment.in_reply_to + && comment_ids.contains(&parent_id) + { + children.entry(parent_id).or_default().push(comment); + continue; + } + roots.push(comment); } - grouped + roots .into_iter() - .map(|(thread_id, mut group)| { - group.sort_by_key(comment_sort_key); - - let root = group.remove(0); - let file_path = root - .path - .clone() - .or_else(|| group.iter().find_map(|comment| comment.path.clone())); - let line = root - .line - .or_else(|| group.iter().find_map(|comment| comment.line)); - + .map(|root| { + let mut replies = Vec::new(); + collect_replies(root.id, &mut children, &mut replies); CommentThread { - thread_id, - file_path, - line, + thread_id: format!("comment-{}", root.id), + file_path: root.path.clone(), + line: root.line, root_comment: to_comment_item(root), - replies: group.into_iter().map(to_comment_item).collect(), + replies: replies.into_iter().map(to_comment_item).collect(), } }) .collect() } -fn thread_key(comment: &ReviewCommentDto) -> String { - match comment.pull_request_review_id { - Some(review_id) => format!( - "review-{review_id}-{path}-{line}-{position}-{commit}", - path = comment.path.as_deref().unwrap_or(""), - line = comment - .line - .map(|value| value.to_string()) - .unwrap_or_default(), - position = comment - .original_position - .or(comment.position) - .map(|value| value.to_string()) - .unwrap_or_default(), - commit = comment - .original_commit_id - .as_deref() - .or(comment.commit_id.as_deref()) - .unwrap_or("") - ), - None => format!("comment-{}", comment.id), +fn collect_replies( + parent_id: i64, + children: &mut BTreeMap>, + out: &mut Vec, +) { + let Some(mut direct_replies) = children.remove(&parent_id) else { + return; + }; + direct_replies.sort_by_key(comment_sort_key); + + for reply in direct_replies { + let reply_id = reply.id; + out.push(reply); + collect_replies(reply_id, children, out); } } diff --git a/tests/normalize_tests.rs b/tests/normalize_tests.rs index af3e4b7..4981539 100644 --- a/tests/normalize_tests.rs +++ b/tests/normalize_tests.rs @@ -55,6 +55,7 @@ fn normalize_groups_replies_and_sorts_threads_by_time() { path: Some("src/main.rs".into()), line: Some(10), pull_request_review_id: Some(7), + in_reply_to: Some(1), original_position: Some(10), position: Some(10), commit_id: Some("abc123".into()), @@ -72,6 +73,7 @@ fn normalize_groups_replies_and_sorts_threads_by_time() { path: Some("src/main.rs".into()), line: Some(10), pull_request_review_id: Some(7), + in_reply_to: None, original_position: Some(10), position: Some(10), commit_id: Some("abc123".into()), @@ -89,6 +91,7 @@ fn normalize_groups_replies_and_sorts_threads_by_time() { path: Some("src/lib.rs".into()), line: Some(22), pull_request_review_id: Some(8), + in_reply_to: None, original_position: Some(22), position: Some(22), commit_id: Some("def456".into()), @@ -170,3 +173,79 @@ fn normalize_groups_replies_and_sorts_threads_by_time() { assert_eq!(doc.threads[1].file_path.as_deref(), Some("src/main.rs")); assert_eq!(doc.threads[1].line, Some(10)); } + +#[test] +fn normalize_does_not_merge_unrelated_comments_on_same_file() { + let bundle = PullBundleDto { + pull: PullDto { + number: 100, + title: "T".into(), + state: "open".into(), + body: None, + user: UserDto { + login: "alice".into(), + }, + base: PullBranchDto { + ref_name: "main".into(), + }, + head: PullBranchDto { + ref_name: "feature/y".into(), + }, + created_at: "2026-04-08T10:00:00Z".into(), + updated_at: "2026-04-08T10:00:00Z".into(), + merged_at: None, + additions: None, + deletions: None, + changed_files: None, + }, + reviews: vec![], + comments: vec![ + ReviewCommentDto { + id: 10, + body: "first line comment".into(), + created_at: "2026-04-08T12:01:00Z".into(), + updated_at: None, + user: UserDto { + login: "bob".into(), + }, + path: Some("src/main.rs".into()), + line: Some(10), + pull_request_review_id: Some(1), + in_reply_to: None, + original_position: Some(10), + position: Some(10), + commit_id: Some("abc123".into()), + original_commit_id: Some("abc123".into()), + diff_hunk: None, + }, + ReviewCommentDto { + id: 11, + body: "second line comment".into(), + created_at: "2026-04-08T12:02:00Z".into(), + updated_at: None, + user: UserDto { + login: "bob".into(), + }, + path: Some("src/main.rs".into()), + line: Some(20), + pull_request_review_id: Some(1), + in_reply_to: None, + original_position: Some(20), + position: Some(20), + commit_id: Some("abc123".into()), + original_commit_id: Some("abc123".into()), + diff_hunk: None, + }, + ], + commits: vec![], + files: vec![], + }; + + let doc = normalize_bundle("org/repo", bundle); + + assert_eq!(doc.threads.len(), 2); + assert_eq!(doc.threads[0].root_comment.id, 10); + assert_eq!(doc.threads[0].replies.len(), 0); + assert_eq!(doc.threads[1].root_comment.id, 11); + assert_eq!(doc.threads[1].replies.len(), 0); +}