From 781038849c26ee6efdc89c3eddbc4348f03b48ea Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Thu, 18 Jun 2026 00:09:22 -0700 Subject: [PATCH 1/2] fix: prevent PR review summary exceeding 65535 len resolves #75 This simply focuses on the length of the patch, because all other parts of the PR review summary will fit under 65535 characters without the patch added. --- cpp-linter/src/clang_tools/mod.rs | 69 +++++++++++++++++++++++++------ cpp-linter/src/rest_client.rs | 6 +-- 2 files changed, 59 insertions(+), 16 deletions(-) diff --git a/cpp-linter/src/clang_tools/mod.rs b/cpp-linter/src/clang_tools/mod.rs index 24483f9..646b045 100644 --- a/cpp-linter/src/clang_tools/mod.rs +++ b/cpp-linter/src/clang_tools/mod.rs @@ -250,6 +250,7 @@ impl ReviewComments { ]; for (tool_name, tool_version) in versions { if let Some(ver) = tool_version { + // If a tool was used, then we know it's version at this point. body.push_str(format!("### Used {tool_name} v{ver}\n").as_str()); } } @@ -286,19 +287,29 @@ impl ReviewComments { // The `full_patch` includes all suggestions that didn't fit in the diff. // It can also contain suggestions that were duplicates of previous reviews. if !self.full_patch.is_empty() { - body.push_str("\n
Click here for "); + let current_len = body.len() + USER_OUTREACH.len(); + let mut patch_prefix = "\n
Click here for ".to_string(); if summary_only { - body.push_str("the full patch of fixes"); + patch_prefix.push_str("the full patch of fixes"); } else { - body.push_str("a patch of fixes outside the diff"); + patch_prefix.push_str("a patch of fixes outside the diff"); + } + patch_prefix.push_str("

\n\n```diff\n"); + let patch_suffix = "```\n\n

\n"; + + if (current_len + patch_prefix.len() + self.full_patch.len() + patch_suffix.len()) + > u16::MAX as usize + { + log::warn!( + "\nThe full patch of fixes is too large to include in the review summary.\n" + ); + body.push_str( + "\nThe full patch of fixes is too large to include in this summary.\n", + ); + } else { + body.push_str(&patch_prefix); + body.push_str(self.full_patch.as_str()); } - body.push_str( - format!( - "

\n\n```diff\n{}```\n\n

\n", - self.full_patch - ) - .as_str(), - ); } else if total_review_comments == 0 { // Only congratulate if there was no reused comments body.push_str("\nNo concerns to report. Great job! :tada:\n"); @@ -377,6 +388,8 @@ mod tests { test_db_parse(tmp_dir.path()).await.unwrap(); } + const PSEUDO_VERSION: Version = Version::new(15, 0, 0); + #[test] fn summarize_reused_reviews() { let comments = vec![ReviewComment { @@ -385,10 +398,9 @@ mod tests { comment: "First comment".to_string(), path: "src/demo.cpp".to_string(), }]; - let pseudo_version = Version::new(15, 0, 0); let clang_versions = ClangVersions { - format_version: Some(pseudo_version.clone()), - tidy_version: Some(pseudo_version), + format_version: Some(PSEUDO_VERSION.clone()), + tidy_version: Some(PSEUDO_VERSION), }; let total_review_comments = 2; let summary_only = false; @@ -400,4 +412,35 @@ mod tests { ); assert!(review_summary.contains("suggestions were duplicates of previous reviews")); } + + #[test] + fn summary_len_truncated() { + let comments = vec![ReviewComment { + line_start: Some(1), + line_end: 1, + comment: "First comment".to_string(), + path: "src/demo.cpp".to_string(), + }]; + let clang_versions = ClangVersions { + format_version: Some(PSEUDO_VERSION.clone()), + tidy_version: Some(PSEUDO_VERSION), + }; + let total_review_comments = 2; + let summary_only = false; + let long_patch = "a".repeat(u16::MAX as usize); + let review_summary = ReviewComments { + full_patch: long_patch, + ..Default::default() + } + .summarize( + &clang_versions, + &comments, + total_review_comments, + summary_only, + ); + assert!( + review_summary + .contains("The full patch of fixes is too large to include in this summary.") + ); + } } diff --git a/cpp-linter/src/rest_client.rs b/cpp-linter/src/rest_client.rs index 2038cc6..4c2d4df 100644 --- a/cpp-linter/src/rest_client.rs +++ b/cpp-linter/src/rest_client.rs @@ -156,13 +156,13 @@ impl RestClient { if feedback_inputs.thread_comments != ThreadComments::Off { // post thread comment for PR or push event - if comment.as_ref().is_none_or(|c| c.len() > 65535) { + if comment.as_ref().is_none_or(|c| c.len() > u16::MAX as usize) { comment = Some(Self::make_comment( files, format_checks_failed, tidy_checks_failed, &clang_versions, - Some(65535), + Some(u16::MAX as u64), )?); } let options = ThreadCommentOptions { @@ -514,7 +514,7 @@ mod test { } let mut files = vec![]; if !is_lgtm { - for _i in 0..65535 { + for _i in 0..u16::MAX { let filename = String::from("tests/demo/demo.cpp"); let mut file = FileObj::new(PathBuf::from(&filename)); let notes = vec![TidyNotification { From fe1d942040fa1a1cf52599c20a8c123931a4d3cc Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Thu, 18 Jun 2026 00:17:05 -0700 Subject: [PATCH 2/2] oops, close the collapsed patch in the summary --- cpp-linter/src/clang_tools/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cpp-linter/src/clang_tools/mod.rs b/cpp-linter/src/clang_tools/mod.rs index 646b045..06faae5 100644 --- a/cpp-linter/src/clang_tools/mod.rs +++ b/cpp-linter/src/clang_tools/mod.rs @@ -301,7 +301,7 @@ impl ReviewComments { > u16::MAX as usize { log::warn!( - "\nThe full patch of fixes is too large to include in the review summary.\n" + "The full patch of fixes is too large to include in the review summary." ); body.push_str( "\nThe full patch of fixes is too large to include in this summary.\n", @@ -309,6 +309,7 @@ impl ReviewComments { } else { body.push_str(&patch_prefix); body.push_str(self.full_patch.as_str()); + body.push_str(patch_suffix); } } else if total_review_comments == 0 { // Only congratulate if there was no reused comments