Fix ChatGPT issue in multi-commit pushes
Fixed a concurrency issue that might occur when executing multiple API
calls to ChatGPT within a tight millisecond timeframe, such as during
multi-commit pushes.
Jira-Id: IT-103
Change-Id: I24d0beef8a4f9a6ffb032637d8c7d34792068361
Signed-off-by: Patrizio <patrizio.gelosi@amarulasolutions.com>
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 cd5f362..c69403e 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/PatchSetReviewer.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/PatchSetReviewer.java
@@ -124,7 +124,7 @@
log.warn("Patch set too large. Skipping review. changeId: {}", changeId);
return String.format(SPLIT_REVIEW_MSG, config.getMaxReviewLines());
}
- return openAiClient.ask(config, patchSet, isCommentEvent);
+ return openAiClient.ask(config, changeId, patchSet, isCommentEvent);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClientPatchSet.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClientPatchSet.java
index 768798d..3d43e4d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClientPatchSet.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/GerritClientPatchSet.java
@@ -90,6 +90,7 @@
String fileDiffJson = forwardGetRequest(uri).replaceAll("^[')\\]}]+", "");
processFileDiff(filename, fileDiffJson);
}
+ diffs.add(String.format("{\"changeId\": \"%s\"}", fullChangeId));
return "[" + String.join(",", diffs) + "]\n";
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/OpenAiClient.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/OpenAiClient.java
index 78747c0..0a2c318 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/OpenAiClient.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/OpenAiClient.java
@@ -8,6 +8,7 @@
import com.googlesource.gerrit.plugins.chatgpt.client.model.ChatCompletionRequest;
import com.googlesource.gerrit.plugins.chatgpt.client.model.ChatCompletionResponseStreamed;
import com.googlesource.gerrit.plugins.chatgpt.client.model.ChatCompletionResponseUnstreamed;
+import com.googlesource.gerrit.plugins.chatgpt.client.model.ChatGptSuggestions;
import com.googlesource.gerrit.plugins.chatgpt.config.Configuration;
import com.googlesource.gerrit.plugins.chatgpt.utils.FileUtils;
import lombok.Getter;
@@ -28,6 +29,7 @@
@Slf4j
@Singleton
public class OpenAiClient {
+ private static final int REVIEW_ATTEMPT_LIMIT = 3;
@Getter
private String requestBody;
private final Gson gson = new GsonBuilder()
@@ -36,25 +38,31 @@
private final HttpClientWithRetry httpClientWithRetry = new HttpClientWithRetry();
private boolean isCommentEvent = false;
- public String ask(Configuration config, String patchSet) throws Exception {
- HttpRequest request = createRequest(config, patchSet);
- log.debug("ChatGPT request: {}", request.toString());
+ public String ask(Configuration config, String changeId, String patchSet) throws Exception {
+ for (int attemptInd = 0; attemptInd < REVIEW_ATTEMPT_LIMIT; attemptInd++) {
+ HttpRequest request = createRequest(config, patchSet);
+ log.debug("ChatGPT request: {}", request.toString());
- HttpResponse<String> response = httpClientWithRetry.execute(request);
+ HttpResponse<String> response = httpClientWithRetry.execute(request);
- String body = response.body();
- log.debug("body: {}", body);
- if (body == null) {
- throw new IOException("ChatGPT response body is null");
+ String body = response.body();
+ log.debug("body: {}", body);
+ if (body == null) {
+ throw new IOException("ChatGPT response body is null");
+ }
+
+ String contentExtracted = extractContent(config, body);
+ if (validateResponse(contentExtracted, changeId, attemptInd)) {
+ return contentExtracted;
+ }
}
-
- return extractContent(config, body);
+ throw new RuntimeException("Failed to receive valid ChatGPT response");
}
- public String ask(Configuration config, String patchSet, boolean isCommentEvent) throws Exception {
+ public String ask(Configuration config, String changeId, String patchSet, boolean isCommentEvent) throws Exception {
this.isCommentEvent = isCommentEvent;
- return this.ask(config, patchSet);
+ return this.ask(config, changeId, patchSet);
}
public String extractContent(Configuration config, String body) throws Exception {
@@ -75,6 +83,19 @@
}
}
+ private boolean validateResponse(String contentExtracted, String changeId, int attemptInd) {
+ ChatGptSuggestions chatGptSuggestions = gson.fromJson(contentExtracted, ChatGptSuggestions.class);
+ String returnedChangeId = chatGptSuggestions.getChangeId();
+ // A response is considered valid if either no changeId is returned or the changeId returned matches the one
+ // provided in the request
+ boolean isValidated = returnedChangeId == null || changeId.equals(returnedChangeId);
+ if (!isValidated) {
+ log.error("ChangedId mismatch error (attempt #{}).\nExpected value: {}\nReturned value: {}", attemptInd,
+ changeId, returnedChangeId);
+ }
+ return isValidated;
+ }
+
private String getResponseContent(List<ChatCompletionBase.ToolCall> toolCalls) {
return toolCalls.get(0).getFunction().getArguments();
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/model/ChatCompletionRequest.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/model/ChatCompletionRequest.java
index a082088..2319051 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/model/ChatCompletionRequest.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/model/ChatCompletionRequest.java
@@ -21,6 +21,8 @@
public static class Message {
private String role;
private String content;
+ // PatchSet changeId passed in the request
+ private String changeId;
}
@Data
@@ -43,6 +45,8 @@
@Data
public static class Properties {
private Property suggestions;
+ // Field `changeId` expected in the response to correspond with the PatchSet changeId in the request
+ private Field changeId;
@Data
public static class Property {
@@ -62,14 +66,14 @@
private Field filename;
private Field lineNumber;
private Field codeSnippet;
-
- @Data
- public static class Field {
- private String type;
- }
}
}
}
+
+ @Data
+ public static class Field {
+ private String type;
+ }
}
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/model/ChatGptSuggestions.java b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/model/ChatGptSuggestions.java
index a3b82cd..84aa45b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/model/ChatGptSuggestions.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/chatgpt/client/model/ChatGptSuggestions.java
@@ -7,4 +7,5 @@
@Data
public class ChatGptSuggestions extends ChatGptSuggestionPoint {
private List<ChatGptSuggestionPoint> suggestions;
+ private String changeId;
}
diff --git a/src/main/resources/Config/prompts.json b/src/main/resources/Config/prompts.json
index 3c3e14d..d20f9a3 100644
--- a/src/main/resources/Config/prompts.json
+++ b/src/main/resources/Config/prompts.json
@@ -4,7 +4,7 @@
"DEFAULT_GPT_USER_PROMPT": "Focus your review on the \"a\" and \"b\" items, but use the \"ab\" items as context to understand the changes better. Provide insights on any potential issues you foresee and suggestions for improvements if necessary, with an emphasis on identifying and addressing concerns rather than highlighting positive aspects.",
"DEFAULT_GPT_USER_PROMPT_JSON": "Each suggestion must be formatted as an individual object within an array. The object will always contain the string field `suggestion`",
"DEFAULT_GPT_CUSTOM_USER_PROMPT_JSON": "along with the key `id`, which corresponds to the `id` value from the related request in the request JSON array",
- "DEFAULT_GPT_USER_PROMPT_JSON_2": "For suggestions that are specific to a certain part of the code, the object should additionally include the keys `filename`, `lineNumber`, and `codeSnippet` to precisely identify the relevant code section.",
+ "DEFAULT_GPT_USER_PROMPT_JSON_2": "Also return back the field `changeId` provided in the corresponding request. For suggestions that are specific to a certain part of the code, the object should additionally include the keys `filename`, `lineNumber`, and `codeSnippet` to precisely identify the relevant code section.",
"DEFAULT_GPT_CUSTOM_USER_PROMPT_1": "I have some requests about the following PatchSet Diff:",
"DEFAULT_GPT_CUSTOM_USER_PROMPT_2": "My requests are given in an array and formatted in JSON:",
"DEFAULT_GPT_COMMIT_MESSAGES_REVIEW_USER_PROMPT": "Also, perform a check on the commit message of the PatchSet. The commit message is provided in the \"content\" field of \"/COMMIT_MSG\" in the same way as the file changes. Ensure that the commit message accurately and succinctly describes the changes made, and verify if it matches the nature and scope of the changes in the PatchSet."
diff --git a/src/main/resources/Config/tools.json b/src/main/resources/Config/tools.json
index 3ce2925..fa2dd7b 100644
--- a/src/main/resources/Config/tools.json
+++ b/src/main/resources/Config/tools.json
@@ -33,10 +33,14 @@
"suggestion"
]
}
+ },
+ "changeId": {
+ "type": "string"
}
},
"required": [
- "suggestions"
+ "suggestions",
+ "changeId"
]
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/integration/CodeReviewPluginIT.java b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/integration/CodeReviewPluginIT.java
index 9f7b241..735c0aa 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/chatgpt/integration/CodeReviewPluginIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/chatgpt/integration/CodeReviewPluginIT.java
@@ -39,7 +39,7 @@
when(config.getGptModel()).thenReturn(Configuration.DEFAULT_GPT_MODEL);
when(config.getGptSystemPrompt()).thenReturn(Configuration.DEFAULT_GPT_SYSTEM_PROMPT);
- String answer = openAiClient.ask(config, "hello");
+ String answer = openAiClient.ask(config, "", "hello");
log.info("answer: {}", answer);
assertNotNull(answer);
}
diff --git a/src/test/resources/__files/chatGptResponseStreamed.txt b/src/test/resources/__files/chatGptResponseStreamed.txt
index 0c2fb8f..930a0cb 100644
--- a/src/test/resources/__files/chatGptResponseStreamed.txt
+++ b/src/test/resources/__files/chatGptResponseStreamed.txt
@@ -199,7 +199,11 @@
data: {"id": "chatcmpl-8gqm87x4kG9YHacjG4o14jm3mFFC8","object": "chat.completion.chunk","created": 1705051034,"model": "gpt-4","system_fingerprint": "fp_668a673906","choices": [{"index": 0,"delta": {"tool_calls": [{"index": 0,"function": {"arguments": " "}}]},"logprobs": null,"finish_reason": null}]}
data: {"id": "chatcmpl-8gqm87x4kG9YHacjG4o14jm3mFFC8","object": "chat.completion.chunk","created": 1705051034,"model": "gpt-4","system_fingerprint": "fp_668a673906","choices": [{"index": 0,"delta": {"tool_calls": [{"index": 0,"function": {"arguments": " }\n"}}]},"logprobs": null,"finish_reason": null}]}
data: {"id": "chatcmpl-8gqm87x4kG9YHacjG4o14jm3mFFC8","object": "chat.completion.chunk","created": 1705051034,"model": "gpt-4","system_fingerprint": "fp_668a673906","choices": [{"index": 0,"delta": {"tool_calls": [{"index": 0,"function": {"arguments": " "}}]},"logprobs": null,"finish_reason": null}]}
-data: {"id": "chatcmpl-8gqm87x4kG9YHacjG4o14jm3mFFC8","object": "chat.completion.chunk","created": 1705051034,"model": "gpt-4","system_fingerprint": "fp_668a673906","choices": [{"index": 0,"delta": {"tool_calls": [{"index": 0,"function": {"arguments": " ]\n"}}]},"logprobs": null,"finish_reason": null}]}
+data: {"id": "chatcmpl-8gqm87x4kG9YHacjG4o14jm3mFFC8","object": "chat.completion.chunk","created": 1705051034,"model": "gpt-4","system_fingerprint": "fp_668a673906","choices": [{"index": 0,"delta": {"tool_calls": [{"index": 0,"function": {"arguments": " ],\n"}}]},"logprobs": null,"finish_reason": null}]}
+data: {"id": "chatcmpl-8gqm87x4kG9YHacjG4o14jm3mFFC8","object": "chat.completion.chunk","created": 1705051034,"model": "gpt-4","system_fingerprint": "fp_668a673906","choices": [{"index": 0,"delta": {"tool_calls": [{"index": 0,"function": {"arguments": " \"changeId\":"}}]},"logprobs": null,"finish_reason": null}]}
+data: {"id": "chatcmpl-8gqm87x4kG9YHacjG4o14jm3mFFC8","object": "chat.completion.chunk","created": 1705051034,"model": "gpt-4","system_fingerprint": "fp_668a673906","choices": [{"index": 0,"delta": {"tool_calls": [{"index": 0,"function": {"arguments": " \"myProject~"}}]},"logprobs": null,"finish_reason": null}]}
+data: {"id": "chatcmpl-8gqm87x4kG9YHacjG4o14jm3mFFC8","object": "chat.completion.chunk","created": 1705051034,"model": "gpt-4","system_fingerprint": "fp_668a673906","choices": [{"index": 0,"delta": {"tool_calls": [{"index": 0,"function": {"arguments": "myBranchName~"}}]},"logprobs": null,"finish_reason": null}]}
+data: {"id": "chatcmpl-8gqm87x4kG9YHacjG4o14jm3mFFC8","object": "chat.completion.chunk","created": 1705051034,"model": "gpt-4","system_fingerprint": "fp_668a673906","choices": [{"index": 0,"delta": {"tool_calls": [{"index": 0,"function": {"arguments": "myChangeId\""}}]},"logprobs": null,"finish_reason": null}]}
data: {"id": "chatcmpl-8gqm87x4kG9YHacjG4o14jm3mFFC8","object": "chat.completion.chunk","created": 1705051034,"model": "gpt-4","system_fingerprint": "fp_668a673906","choices": [{"index": 0,"delta": {"tool_calls": [{"index": 0,"function": {"arguments": "}"}}]},"logprobs": null,"finish_reason": null}]}
data: {"id": "chatcmpl-8gqm87x4kG9YHacjG4o14jm3mFFC8","object": "chat.completion.chunk","created": 1705051034,"model": "gpt-4","system_fingerprint": "fp_668a673906","choices": [{"index": 0,"delta": {},"logprobs": null,"finish_reason": null}]}
data: [DONE]