Provide complete file diffs to ChatGPT
The full file differences are forwarded to ChatGPT, rather than just
the revision content with a few context lines before and after the
changes. This enhances the response quality by offering comprehensive
file context.
Additionally, introduced the 'maxReviewFileSize' configuration option
to ensure only files of a certain size or smaller are reviewed.
Jira-Id: IT-103
Change-Id: I2b58884e2a2569b57cec038f8ec8157a173c8276
Signed-off-by: Patrizio <patrizio.gelosi@amarulasolutions.com>
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClient.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClient.java
index f7cecce..205a29e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClient.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClient.java
@@ -4,6 +4,7 @@
import com.google.gson.Gson;
import com.google.inject.Singleton;
import com.googlesource.gerrit.plugins.chatgpt.config.Configuration;
+import com.google.gson.reflect.TypeToken;
import lombok.extern.slf4j.Slf4j;
import org.apache.http.entity.ContentType;
@@ -12,9 +13,8 @@
import java.net.http.HttpRequest;
import java.net.http.HttpResponse;
import java.nio.charset.StandardCharsets;
-import java.util.Base64;
-import java.util.HashMap;
-import java.util.Map;
+import java.util.*;
+import java.lang.reflect.Type;
import static java.net.HttpURLConnection.HTTP_OK;
@@ -24,12 +24,14 @@
private final Gson gson = new Gson();
private final HttpClientWithRetry httpClientWithRetry = new HttpClientWithRetry();
- public String getPatchSet(Configuration config, String fullChangeId) throws Exception {
+ private List<String> getAffectedFiles(Configuration config, String fullChangeId) throws Exception {
+ URI uri = URI.create(config.getGerritAuthBaseUrl()
+ + UriResourceLocator.gerritPatchSetFilesUri(fullChangeId));
+ log.debug("patchSet uri: {}", uri.toString());
HttpRequest request = HttpRequest.newBuilder()
.header(HttpHeaders.AUTHORIZATION, generateBasicAuth(config.getGerritUserName(),
config.getGerritPassword()))
- .uri(URI.create(config.getGerritAuthBaseUrl()
- + UriResourceLocator.gerritPatchSetUri(fullChangeId)))
+ .uri(uri)
.build();
HttpResponse<String> response = httpClientWithRetry.execute(request);
@@ -41,7 +43,60 @@
String responseBody = response.body();
log.info("Successfully obtained patch. Decoding response body.");
- return new String(Base64.getDecoder().decode(responseBody));
+
+ Type listType = new TypeToken<Map<String, Map<String, String>>>() {}.getType();
+ Map<String, Map<String, String>> map = gson.fromJson(responseBody, listType);
+ List<String> files = new ArrayList<>();
+ for (Map.Entry<String, Map<String, String>> file : map.entrySet()) {
+ String filename = file.getKey();
+ if (!filename.equals("/COMMIT_MSG")) {
+ Integer size = Integer.valueOf(file.getValue().get("size"));
+ if (size > config.getMaxReviewFileSize()) {
+ log.info("File '{}' not reviewed because its size exceeds the fixed maximum allowable size.",
+ filename);
+ }
+ else {
+ files.add(filename);
+ }
+ }
+ }
+
+ return files;
+ }
+
+ private String getFileDiffs(Configuration config, String fullChangeId, List<String> files) throws Exception {
+ List<String> diffs = new ArrayList<>();
+ for (String filename : files) {
+ URI uri = URI.create(config.getGerritAuthBaseUrl()
+ + UriResourceLocator.gerritPatchSetFilesUri(fullChangeId)
+ + UriResourceLocator.gerritDiffPostfixUri(filename));
+ log.debug("Diff uri: {}", uri.toString());
+ HttpRequest request = HttpRequest.newBuilder()
+ .header(HttpHeaders.AUTHORIZATION, generateBasicAuth(config.getGerritUserName(),
+ config.getGerritPassword()))
+ .uri(uri)
+ .build();
+
+ HttpResponse<String> response = httpClientWithRetry.execute(request);
+
+ if (response.statusCode() != HTTP_OK) {
+ log.error("Failed to get patch. Response: {}", response);
+ throw new IOException("Failed to get patch from Gerrit");
+ }
+
+ diffs.add(response.body().replaceAll("^[')\\]}']+", ""));
+ }
+ return "[" + String.join(",", diffs) + "]";
+ }
+
+ public String getPatchSet(Configuration config, String fullChangeId) throws Exception {
+ List<String> files = getAffectedFiles(config, fullChangeId);
+ log.debug("Patch files: {}", files.toString());
+
+ String fileDiffs = getFileDiffs(config, fullChangeId, files);
+ log.debug("File diffs: {}", fileDiffs);
+
+ return fileDiffs;
}
private String generateBasicAuth(String username, String password) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/UriResourceLocator.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/UriResourceLocator.java
index 6f58e28..6041726 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/UriResourceLocator.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/UriResourceLocator.java
@@ -10,8 +10,12 @@
return "/a";
}
- public static String gerritPatchSetUri(String fullChangeId) {
- return "/changes/" + fullChangeId + "/revisions/current/patch";
+ public static String gerritDiffPostfixUri(String filename) {
+ return "/" + filename + "/diff";
+ }
+
+ public static String gerritPatchSetFilesUri(String fullChangeId) {
+ return "/changes/" + fullChangeId + "/revisions/current/files";
}
public static String gerritCommentUri(String fullChangeId) {
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 13a8f1f..2a000c7 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
@@ -11,7 +11,9 @@
public static final String OPENAI_DOMAIN = "https://api.openai.com";
public static final String DEFAULT_GPT_MODEL = "gpt-3.5-turbo";
- public static final String DEFAULT_GPT_PROMPT = "Act as a Code Review Helper, please review this patch set: ";
+ public static final String DEFAULT_GPT_PROMPT = "Act as a Code Review Helper. Review only the \"a\" (lines " +
+ "removed) and \"b\" (lines added) items of the following diff, using the lines in the \"ab\" items as " +
+ "context. ";
public static final String NOT_CONFIGURED_ERROR_MSG = "%s is not configured";
public static final String KEY_GPT_PROMPT = "gptPrompt";
private static final String DEFAULT_GPT_TEMPERATURE = "1";
@@ -21,6 +23,7 @@
private static final boolean DEFAULT_PATCH_SET_REDUCTION = false;
private static final boolean DEFAULT_PROJECT_ENABLE = false;
private static final int DEFAULT_MAX_REVIEW_LINES = 1000;
+ private static final int DEFAULT_MAX_REVIEW_FILE_SIZE = 10000;
private static final String KEY_GPT_TOKEN = "gptToken";
private static final String KEY_GERRIT_AUTH_BASE_URL = "gerritAuthBaseUrl";
private static final String KEY_GERRIT_USERNAME = "gerritUserName";
@@ -34,6 +37,7 @@
private static final String KEY_ENABLED_PROJECTS = "enabledProjects";
private static final String KEY_PATCH_SET_REDUCTION = "patchSetReduction";
private static final String KEY_MAX_REVIEW_LINES = "maxReviewLines";
+ private static final String KEY_MAX_REVIEW_FILE_SIZE = "maxReviewFileSize";
private final Map<String, Object> configsDynamically = Maps.newHashMap();
private final PluginConfig globalConfig;
private final PluginConfig projectConfig;
@@ -106,6 +110,10 @@
return getInt(KEY_MAX_REVIEW_LINES, DEFAULT_MAX_REVIEW_LINES);
}
+ public int getMaxReviewFileSize() {
+ return getInt(KEY_MAX_REVIEW_FILE_SIZE, DEFAULT_MAX_REVIEW_FILE_SIZE);
+ }
+
private String getValidatedOrThrow(String key) {
String value = projectConfig.getString(key);
if (value == null) {
diff --git a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTest.java b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTest.java
index f1254e4..71fccb3 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/ChatGptReviewTest.java
@@ -36,7 +36,8 @@
import java.util.concurrent.ExecutionException;
import static com.googlesource.gerrit.plugins.chatgpt.client.UriResourceLocator.gerritCommentUri;
-import static com.googlesource.gerrit.plugins.chatgpt.client.UriResourceLocator.gerritPatchSetUri;
+import static com.googlesource.gerrit.plugins.chatgpt.client.UriResourceLocator.gerritDiffPostfixUri;
+import static com.googlesource.gerrit.plugins.chatgpt.client.UriResourceLocator.gerritPatchSetFilesUri;
import static com.googlesource.gerrit.plugins.chatgpt.listener.EventListenerHandler.buildFullChangeId;
import static java.net.HttpURLConnection.HTTP_OK;
import static org.mockito.Mockito.mock;
@@ -67,17 +68,26 @@
when(config.getGptTemperature()).thenReturn(1.0);
when(config.getGptStreamOutput()).thenReturn(true);
when(config.getMaxReviewLines()).thenReturn(500);
+ when(config.getMaxReviewFileSize()).thenReturn(10000);
when(config.getEnabledProjects()).thenReturn("");
when(config.isProjectEnable()).thenReturn(true);
}
private void setupMockRequests() {
- // Mocks the behavior of the getPatchSet request
- WireMock.stubFor(WireMock.get(gerritPatchSetUri(buildFullChangeId(PROJECT_NAME, BRANCH_NAME, CHANGE_ID)))
+ // Mocks the behavior of the gerritPatchSetFiles request
+ WireMock.stubFor(WireMock.get(gerritPatchSetFilesUri(buildFullChangeId(PROJECT_NAME, BRANCH_NAME, CHANGE_ID)))
.willReturn(WireMock.aResponse()
.withStatus(HTTP_OK)
.withHeader(HttpHeaders.CONTENT_TYPE, ContentType.APPLICATION_JSON.toString())
- .withBody(Base64.getEncoder().encodeToString("myPatch".getBytes()))));
+ .withBodyFile("gerritPatchSetFiles.json")));
+
+ // Mocks the behavior of the gerritPatchSet diff request
+ WireMock.stubFor(WireMock.get(gerritPatchSetFilesUri(buildFullChangeId(PROJECT_NAME, BRANCH_NAME, CHANGE_ID)) +
+ gerritDiffPostfixUri("test_file.py"))
+ .willReturn(WireMock.aResponse()
+ .withStatus(HTTP_OK)
+ .withHeader(HttpHeaders.CONTENT_TYPE, ContentType.APPLICATION_JSON.toString())
+ .withBodyFile("gerritPatchSetDiff.json")));
// Mocks the behavior of the askGpt request
byte[] gptAnswer = Base64.getDecoder().decode("ZGF0YTogeyJpZCI6ImNoYXRjbXBsLTdSZDVOYVpEOGJNVTRkdnBVV2" +
diff --git a/src/test/resources/__files/gerritPatchSetDiff.json b/src/test/resources/__files/gerritPatchSetDiff.json
new file mode 100644
index 0000000..a14a217
--- /dev/null
+++ b/src/test/resources/__files/gerritPatchSetDiff.json
@@ -0,0 +1,55 @@
+{
+ "meta_a": {
+ "name": "test_file.py",
+ "content_type": "text/x-python",
+ "lines": 42
+ },
+ "meta_b": {
+ "name": "test_file.py",
+ "content_type": "text/x-python",
+ "lines": 42
+ },
+ "change_type": "MODIFIED",
+ "diff_header": [
+ "diff --git a/test_file.py b/test_file.py",
+ "index 3af5d07..0c975d6 100644",
+ "--- a/test_file.py",
+ "+++ b/test_file.py"
+ ],
+ "content": [
+ {
+ "ab": [
+ "from typing import Any, Callable, Type, Union",
+ "",
+ "__all__ = [\"importclass\", \"preprocess_classes\", \"TypeClassOrPath\"]",
+ "",
+ "TypeClassOrPath = Union[Type, str]",
+ "",
+ "",
+ "def importclass(",
+ " module_name: str,",
+ " class_name: Union[str, None] = None",
+ ") -> Type:",
+ " \"\"\"",
+ " Dynamically import a class from a specified module.",
+ "",
+ " :param module_name: The name of the module to import.",
+ " :param class_name: The name of the class in the module to import. Defaults to None.",
+ " :return: The dynamically imported class.",
+ " \"\"\"",
+ " if not class_name:",
+ " module_name, class_name = module_name.rsplit('.', 1)",
+ " loaded_module = __import__(module_name, fromlist=[class_name])"
+ ]
+ },
+ {
+ "a": [
+ " return getattr(loaded_module, class_name)"
+ ],
+ "b": [
+ " return getattr(loaded_module, class_name)"
+ ],
+ "common": true
+ }
+ ]
+}
\ No newline at end of file
diff --git a/src/test/resources/__files/gerritPatchSetFiles.json b/src/test/resources/__files/gerritPatchSetFiles.json
new file mode 100644
index 0000000..40885b5
--- /dev/null
+++ b/src/test/resources/__files/gerritPatchSetFiles.json
@@ -0,0 +1,16 @@
+{
+ "/COMMIT_MSG": {
+ "status": "A",
+ "lines_inserted": 9,
+ "size_delta": 355,
+ "size": 355
+ },
+ "test_file.py":{
+ "old_mode":33188,
+ "new_mode":33188,
+ "lines_inserted":1,
+ "lines_deleted":1,
+ "size_delta":11,
+ "size":6737
+ }
+}
\ No newline at end of file
