Hide low-scoring review comments
Implemented a filter to hide positive or neutral review comments. The
filter is controlled by the `filterNegativeComments` configuration
setting, with the threshold for filtering customizable through the
`filterCommentsBelowScore` setting.
Jira-Id: IT-103
Change-Id: Iafb898d1fd35caa1a0579c0be1c78ed1a9ace227
Signed-off-by: Patrizio <patrizio.gelosi@amarulasolutions.com>
diff --git a/README.md b/README.md
index efd50d2..d18357e 100644
--- a/README.md
+++ b/README.md
@@ -149,6 +149,11 @@
.sh, .vb, .bat".
- `enabledVoting`: Initially disabled (false). If set to true, allows ChatGPT to cast a vote on each reviewed Patch Set
by assigning a score.
+- `filterNegativeComments`: Activated by default (true), ensuring only negative review comments (scored below the
+ `filterCommentsBelowScore` threshold outlined further) are displayed initially. Disabling this setting (false) will
+ also show positive and neutral comments.
+- `filterCommentsBelowScore`: With `filterNegativeComments` active, review comments with a score at or above this
+ setting's value are hidden (default is 0).
- `patchSetCommentsAsResolved`: Initially set to false, this option leaves ChatGPT's Patch Set comments as unresolved,
inviting further discussion. If activated, it marks ChatGPT's Patch Set comments as resolved.
- `inlineCommentsAsResolved`: Initially set to false, this option leaves ChatGPT's inline comments as unresolved,
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/PatchSetReviewer.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/PatchSetReviewer.java
index 22a6464..4c3b2a5 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/PatchSetReviewer.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/PatchSetReviewer.java
@@ -27,6 +27,7 @@
private final GerritClient gerritClient;
private final ChatGptClient chatGptClient;
+ private Configuration config;
private GerritCommentRange gerritCommentRange;
private List<ReviewBatch> reviewBatches;
private List<GerritComment> commentProperties;
@@ -39,6 +40,7 @@
}
public void review(Configuration config, GerritChange change) throws Exception {
+ this.config = config;
reviewBatches = new ArrayList<>();
reviewScores = new ArrayList<>();
commentProperties = gerritClient.getClientData(change).getCommentProperties();
@@ -51,11 +53,11 @@
}
DynamicSettings.update(config, change, gerritClient);
- String reviewReply = getReviewReply(config, change, patchSet);
+ String reviewReply = getReviewReply(change, patchSet);
log.debug("ChatGPT response: {}", reviewReply);
retrieveReviewBatches(reviewReply, change);
- gerritClientReview.setReview(change.getFullChangeId(), reviewBatches, getReviewScore(config));
+ gerritClientReview.setReview(change.getFullChangeId(), reviewBatches, getReviewScore());
}
private void setCommentBatchMap(ReviewBatch batchMap, Integer batchID) {
@@ -94,7 +96,9 @@
if (!replyItem.isConflicting() && score != null) {
reviewScores.add(score);
}
- if (shouldFilterReplies && (replyItem.isRepeated() || replyItem.isConflicting())) {
+ if (shouldFilterReplies && (replyItem.isRepeated() || replyItem.isConflicting() ||
+ config.getFilterNegativeComments() && score != null &&
+ score >= config.getFilterCommentsBelowScore())) {
continue;
}
ReviewBatch batchMap = new ReviewBatch();
@@ -109,7 +113,7 @@
}
}
- private String getReviewReply(Configuration config, GerritChange change, String patchSet) throws Exception {
+ private String getReviewReply(GerritChange change, String patchSet) throws Exception {
List<String> patchLines = Arrays.asList(patchSet.split("\n"));
if (patchLines.size() > config.getMaxReviewLines()) {
log.warn("Patch set too large. Skipping review. changeId: {}", change.getFullChangeId());
@@ -118,7 +122,7 @@
return chatGptClient.ask(config, change, patchSet);
}
- private Integer getReviewScore(Configuration config) {
+ private Integer getReviewScore() {
return config.isVotingEnabled() && !reviewScores.isEmpty() ? Collections.min(reviewScores) : null;
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/prompt/ChatGptPrompt.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/prompt/ChatGptPrompt.java
index 29c37e8..f151fe3 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/prompt/ChatGptPrompt.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/prompt/ChatGptPrompt.java
@@ -83,7 +83,7 @@
public String getPatchSetReviewUserPrompt() {
List<String> attributes = new ArrayList<>(PATCH_SET_REVIEW_REPLY_ATTRIBUTES);
- if (config.isVotingEnabled()) {
+ if (config.isVotingEnabled() || config.getFilterNegativeComments()) {
updateScoreDescription();
}
else {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/config/Configuration.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/config/Configuration.java
index c71b3ee..1593e98 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/config/Configuration.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/config/Configuration.java
@@ -63,6 +63,8 @@
private static final int DEFAULT_MAX_REVIEW_LINES = 1000;
private static final int DEFAULT_MAX_REVIEW_FILE_SIZE = 10000;
private static final boolean DEFAULT_ENABLED_VOTING = false;
+ private static final boolean DEFAULT_FILTER_NEGATIVE_COMMENTS = true;
+ private static final int DEFAULT_FILTER_COMMENTS_BELOW_SCORE = 0;
private static final int DEFAULT_VOTING_MIN_SCORE = -1;
private static final int DEFAULT_VOTING_MAX_SCORE = 1;
private static final boolean DEFAULT_INLINE_COMMENTS_AS_RESOLVED = false;
@@ -99,6 +101,8 @@
private static final String KEY_MAX_REVIEW_FILE_SIZE = "maxReviewFileSize";
private static final String KEY_ENABLED_FILE_EXTENSIONS = "enabledFileExtensions";
private static final String KEY_ENABLED_VOTING = "enabledVoting";
+ private static final String KEY_FILTER_NEGATIVE_COMMENTS = "filterNegativeComments";
+ private static final String KEY_FILTER_COMMENTS_BELOW_SCORE = "filterCommentsBelowScore";
private static final String KEY_INLINE_COMMENTS_AS_RESOLVED = "inlineCommentsAsResolved";
private static final String KEY_PATCH_SET_COMMENTS_AS_RESOLVED = "patchSetCommentsAsResolved";
private static final String KEY_IGNORE_OUTDATED_INLINE_COMMENTS = "ignoreOutdatedInlineComments";
@@ -206,6 +210,14 @@
return getBoolean(KEY_ENABLED_VOTING, DEFAULT_ENABLED_VOTING);
}
+ public boolean getFilterNegativeComments() {
+ return getBoolean(KEY_FILTER_NEGATIVE_COMMENTS, DEFAULT_FILTER_NEGATIVE_COMMENTS);
+ }
+
+ public int getFilterCommentsBelowScore() {
+ return getInt(KEY_FILTER_COMMENTS_BELOW_SCORE, DEFAULT_FILTER_COMMENTS_BELOW_SCORE);
+ }
+
public int getVotingMinScore() {
return getInt(KEY_VOTING_MIN_SCORE, DEFAULT_VOTING_MIN_SCORE);
}
diff --git a/src/test/resources/__files/gerritPatchSetReview.json b/src/test/resources/__files/gerritPatchSetReview.json
index ba22fe5..bf5f29a 100644
--- a/src/test/resources/__files/gerritPatchSetReview.json
+++ b/src/test/resources/__files/gerritPatchSetReview.json
@@ -1 +1 @@
-{"comments":{"test_file.py":[{"unresolved":true,"line":19,"range":{"start_line":19,"end_line":20,"start_character":4,"end_character":60},"message":"Confirm that the method \u0027importclass\u0027 is meant to change its behavior when \u0027class_name\u0027 is None. The new lines suggest \u0027class_name\u0027 will be derived from the \u0027module_name\u0027 in such cases, which can have unintended effects if not explicitly intended."},{"unresolved":true,"line":20,"range":{"start_line":20,"end_line":20,"start_character":8,"end_character":60},"message":"The added check to determine if \u0027class_name\u0027 is None seems to modify the \u0027module_name\u0027 by splitting it and taking the last element. There should be an assignment to \u0027class_name\u0027 since the class to be imported is meant to be the last part of \u0027module_name\u0027 after splitting. Additionally, it would be beneficial to include a comment like:\n\\# last element is..."},{"unresolved":true,"line":1,"range":{"start_line":1,"end_line":1,"start_character":0,"end_character":17},"message":"The code line \u0027from types import Any, Callable, ...\u0027 should use \u0027typing\u0027 for imports instead of \u0027types\u0027."},{"unresolved":true,"line":21,"range":{"start_line":21,"end_line":21,"start_character":4,"end_character":67},"message":"There is a typo in the import statement. The correct function should be \u0027\\__import\\__\u0027 from the \u0027importlib\u0027 module, not \u0027importclass\u0027 which does not exist. Correct code:\n\n```\nloaded_module \u003d import_module(module_name, fromlist\u003d[class_name])\n```\n"}],"/PATCHSET_LEVEL":[{"unresolved":true,"message":"The commit message \u0027Test Commit Message\u0027 is too vague and does not provide information about the specific changes made. A more detailed message is necessary to understand what has been fixed."}]},"labels":{"Code-Review":-1}}
\ No newline at end of file
+{"comments":{"test_file.py":[{"unresolved":true,"line":20,"range":{"start_line":20,"end_line":20,"start_character":8,"end_character":60},"message":"The added check to determine if \u0027class_name\u0027 is None seems to modify the \u0027module_name\u0027 by splitting it and taking the last element. There should be an assignment to \u0027class_name\u0027 since the class to be imported is meant to be the last part of \u0027module_name\u0027 after splitting. Additionally, it would be beneficial to include a comment like:\n\\# last element is..."},{"unresolved":true,"line":1,"range":{"start_line":1,"end_line":1,"start_character":0,"end_character":17},"message":"The code line \u0027from types import Any, Callable, ...\u0027 should use \u0027typing\u0027 for imports instead of \u0027types\u0027."},{"unresolved":true,"line":21,"range":{"start_line":21,"end_line":21,"start_character":4,"end_character":67},"message":"There is a typo in the import statement. The correct function should be \u0027\\__import\\__\u0027 from the \u0027importlib\u0027 module, not \u0027importclass\u0027 which does not exist. Correct code:\n\n```\nloaded_module \u003d import_module(module_name, fromlist\u003d[class_name])\n```\n"}],"/PATCHSET_LEVEL":[{"unresolved":true,"message":"The commit message \u0027Test Commit Message\u0027 is too vague and does not provide information about the specific changes made. A more detailed message is necessary to understand what has been fixed."}]},"labels":{"Code-Review":-1}}
\ No newline at end of file
