Follow up PR with new detect bad practices for individual PR, better LLM prompt, responsive spinners, and ready to merge trigger for detection

server/application-server/openapi.yaml

@@ -288,6 +288,27 @@ paths:
             application/json:
               schema:
                 $ref: "#/components/schemas/Message"
+  /activity/{pullRequestId}/badpractices/:
+    post:
+      tags:
+      - activity
+      operationId: detectBadPracticesForPullRequest
+      parameters:
+      - name: pullRequestId
+        in: path
+        required: true
+        schema:
+          type: integer
+          format: int64
+      responses:
+        "200":
+          description: OK
+          content:
+            application/json:
+              schema:
+                type: array
+                items:
+                  $ref: "#/components/schemas/PullRequestBadPractice"
   /activity/{login}/badpractices:
     post:
       tags:

server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/ActivityController.java

@@ -22,7 +22,13 @@ public ResponseEntity<ActivityDTO> getActivityByUser(@PathVariable String login)
 
     @PostMapping("/{login}/badpractices")
     public ResponseEntity<List<PullRequestBadPracticeDTO>> detectBadPracticesByUser(@PathVariable String login) {
-        List<PullRequestBadPracticeDTO> badPractices = activityService.detectBadPractices(login);
+        List<PullRequestBadPracticeDTO> badPractices = activityService.detectBadPracticesForUser(login);
         return ResponseEntity.ok(badPractices);
     }
+
+    @PostMapping("{pullRequestId}/badpractices/")
+    public ResponseEntity<List<PullRequestBadPracticeDTO>> detectBadPracticesForPullRequest(@PathVariable Long pullRequestId) {
+        List<PullRequestBadPracticeDTO> badPractice = activityService.detectBadPracticesForPullRequest(pullRequestId);
+        return ResponseEntity.ok(badPractice);
+    }
 }

server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/ActivityService.java

@@ -34,7 +34,6 @@ public class ActivityService {
     @Autowired
     private UserRepository userRepository;
 
-    @Transactional
     public ActivityDTO getActivity(String login) {
         logger.info("Getting activity for user with login: {}", login);
 
@@ -48,7 +47,6 @@ public ActivityDTO getActivity(String login) {
 
         Map<PullRequest, List<PullRequestBadPracticeDTO>> pullRequestBadPracticesMap = openPulLRequestBadPractices
             .stream()
-            .filter(pullRequestBadPractice -> !pullRequestBadPractice.isResolved())
             .collect(
                 Collectors.groupingBy(
                     PullRequestBadPractice::getPullrequest,
@@ -71,24 +69,32 @@ public ActivityDTO getActivity(String login) {
         return new ActivityDTO(openPullRequestsWithBadPractices);
     }
 
-    public List<PullRequestBadPracticeDTO> detectBadPractices(String login) {
+    public List<PullRequestBadPracticeDTO> detectBadPracticesForUser(String login) {
         logger.info("Detecting bad practices for user with login: {}", login);
 
         List<PullRequest> pullRequests = pullRequestRepository.findAssignedByLoginAndStates(
             login,
             Set.of(Issue.State.OPEN)
         );
 
-        List<PullRequestBadPractice> existingBadPractices = pullRequestBadPracticeRepository.findAssignedByLoginAndOpen(
-            login
-        );
-        existingBadPractices.forEach(existingBadPractice -> existingBadPractice.setResolved(true));
-        pullRequestBadPracticeRepository.saveAll(existingBadPractices);
-
         List<PullRequestBadPractice> detectedBadPractices = new ArrayList<>();
         for (PullRequest pullRequest : pullRequests) {
             detectedBadPractices.addAll(pullRequestBadPracticeDetector.detectAndSyncBadPractices(pullRequest));
         }
         return detectedBadPractices.stream().map(PullRequestBadPracticeDTO::fromPullRequestBadPractice).toList();
     }
+
+    public List<PullRequestBadPracticeDTO> detectBadPracticesForPullRequest(Long pullRequestId) {
+        logger.info("Detecting bad practices for PR: {}", pullRequestId);
+
+        PullRequest pullRequest = pullRequestRepository.findById(pullRequestId).orElse(null);
+        if (pullRequest == null) {
+            throw new IllegalArgumentException("Pull request " + pullRequestId + " not found");
+        }
+
+        List<PullRequestBadPractice> detectedBadPractices = pullRequestBadPracticeDetector.detectAndSyncBadPractices(
+            pullRequest
+        );
+        return detectedBadPractices.stream().map(PullRequestBadPracticeDTO::fromPullRequestBadPractice).toList();
+    }
 }

server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/badpracticedetector/BadPracticeDetectorScheduler.java

@@ -0,0 +1,28 @@
+package de.tum.in.www1.hephaestus.activity.badpracticedetector;
+
+import de.tum.in.www1.hephaestus.gitprovider.label.Label;
+import de.tum.in.www1.hephaestus.gitprovider.pullrequest.PullRequest;
+import java.util.Set;
+import org.springframework.stereotype.Component;
+
+@Component
+public class BadPracticeDetectorScheduler {
+
+    private static final String READY_TO_MERGE = "ready to merge";
+
+    public void detectBadPracticeForPrIfReadyToMerge(
+        PullRequest pullRequest,
+        Set<Label> oldLabels,
+        Set<Label> newLabels
+    ) {
+        if (
+            newLabels.stream().anyMatch(label -> READY_TO_MERGE.equals(label.getName())) &&
+            oldLabels.stream().noneMatch(label -> READY_TO_MERGE.equals(label.getName()))
+        ) {
+            BadPracticeDetectorTask badPracticeDetectorTask = new BadPracticeDetectorTask();
+            badPracticeDetectorTask.setPullRequest(pullRequest);
+            Thread thread = new Thread(badPracticeDetectorTask);
+            thread.start();
+        }
+    }
+}

server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/badpracticedetector/BadPracticeDetectorTask.java

@@ -0,0 +1,23 @@
+package de.tum.in.www1.hephaestus.activity.badpracticedetector;
+
+import de.tum.in.www1.hephaestus.gitprovider.pullrequest.PullRequest;
+import lombok.Getter;
+import lombok.Setter;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.stereotype.Component;
+
+@Component
+@Getter
+@Setter
+public class BadPracticeDetectorTask implements Runnable {
+
+    @Autowired
+    private PullRequestBadPracticeDetector pullRequestBadPracticeDetector;
+
+    private PullRequest pullRequest;
+
+    @Override
+    public void run() {
+        pullRequestBadPracticeDetector.detectAndSyncBadPractices(pullRequest);
+    }
+}

server/application-server/src/main/java/de/tum/in/www1/hephaestus/activity/badpracticedetector/PullRequestBadPracticeDetector.java

@@ -33,27 +33,57 @@ public class PullRequestBadPracticeDetector {
      */
     public List<PullRequestBadPractice> detectAndSyncBadPractices(PullRequest pullRequest) {
         logger.info("Detecting bad practices for pull request: {}", pullRequest.getId());
+
+        List<PullRequestBadPractice> existingBadPractices = pullRequestBadPracticeRepository.findByPullRequestId(
+            pullRequest.getId()
+        );
+
         DetectorRequest detectorRequest = new DetectorRequest();
         detectorRequest.setDescription(pullRequest.getBody());
         detectorRequest.setTitle(pullRequest.getTitle());
+        detectorRequest.setBadPractices(
+            existingBadPractices.stream().map(this::convertToIntelligenceBadPractice).toList()
+        );
         DetectorResponse detectorResponse = detectorApi.detectDetectorPost(detectorRequest);
 
         List<PullRequestBadPractice> detectedBadPractices = new LinkedList<>();
 
+        // Check if there are returned bad practices in the response with the same title as an existing bad practice
         for (BadPractice badPractice : detectorResponse.getBadPractices()) {
-            detectedBadPractices.add(handleDetectedBadPractices(pullRequest, badPractice));
+            boolean exists = false;
+            for (PullRequestBadPractice existingBadPractice : existingBadPractices) {
+                if (existingBadPractice.getTitle().equals(badPractice.getTitle())) {
+                    existingBadPractice.setDescription(badPractice.getDescription());
+                    existingBadPractice.setResolved(badPractice.getResolved());
+                    detectedBadPractices.add(pullRequestBadPracticeRepository.save(existingBadPractice));
+                    exists = true;
+                    break;
+                }
+            }
+            if (!exists) {
+                detectedBadPractices.add(saveDetectedBadPractices(pullRequest, badPractice));
+            }
         }
 
         logger.info("Detected {} bad practices for pull request: {}", detectedBadPractices.size(), pullRequest.getId());
         return detectedBadPractices;
     }
 
-    protected PullRequestBadPractice handleDetectedBadPractices(PullRequest pullRequest, BadPractice badPractice) {
+    protected PullRequestBadPractice saveDetectedBadPractices(PullRequest pullRequest, BadPractice badPractice) {
+
         PullRequestBadPractice pullRequestBadPractice = new PullRequestBadPractice();
         pullRequestBadPractice.setTitle(badPractice.getTitle());
         pullRequestBadPractice.setDescription(badPractice.getDescription());
         pullRequestBadPractice.setPullrequest(pullRequest);
-        pullRequestBadPractice.setResolved(false);
+        pullRequestBadPractice.setResolved(badPractice.getResolved());
         return pullRequestBadPracticeRepository.save(pullRequestBadPractice);
     }
+
+    private BadPractice convertToIntelligenceBadPractice(PullRequestBadPractice pullRequestBadPractice) {
+        BadPractice badPractice = new BadPractice();
+        badPractice.setTitle(pullRequestBadPractice.getTitle());
+        badPractice.setDescription(pullRequestBadPractice.getDescription());
+        badPractice.setResolved(pullRequestBadPractice.isResolved());
+        return badPractice;
+    }
 }

server/application-server/src/main/java/de/tum/in/www1/hephaestus/gitprovider/pullrequest/PullRequestRepository.java

@@ -21,6 +21,7 @@ SELECT MIN(p.createdAt)
     )
     Optional<OffsetDateTime> firstContributionByAuthorLogin(@Param("authorLogin") String authorLogin);
 
+    @Transactional
     @Query(
         """
         SELECT p

server/application-server/src/main/java/de/tum/in/www1/hephaestus/gitprovider/pullrequest/github/GitHubPullRequestSyncService.java

@@ -1,5 +1,6 @@
 package de.tum.in.www1.hephaestus.gitprovider.pullrequest.github;
 
+import de.tum.in.www1.hephaestus.activity.badpracticedetector.BadPracticeDetectorScheduler;
 import de.tum.in.www1.hephaestus.gitprovider.common.DateUtil;
 import de.tum.in.www1.hephaestus.gitprovider.label.Label;
 import de.tum.in.www1.hephaestus.gitprovider.label.LabelRepository;
@@ -62,6 +63,9 @@ public class GitHubPullRequestSyncService {
     @Autowired
     private GitHubUserConverter userConverter;
 
+    @Autowired
+    private BadPracticeDetectorScheduler badPracticeDetectorScheduler;
+
     /**
      * Synchronizes all pull requests from the specified GitHub repositories.
      *
@@ -233,6 +237,7 @@ public PullRequest processPullRequest(GHPullRequest ghPullRequest) {
                 });
             resultLabels.add(resultLabel);
         });
+        badPracticeDetectorScheduler.detectBadPracticeForPrIfReadyToMerge(result, result.getLabels(), resultLabels);
         result.getLabels().clear();
         result.getLabels().addAll(resultLabels);
 

server/application-server/src/main/java/de/tum/in/www1/hephaestus/intelligenceservice/api/DetectorApi.java

@@ -40,7 +40,7 @@ public DetectorApi(ApiClient apiClient) {
     }
 
     /**
-     * Detect bad practices given rules.
+     * Detect bad practices for given pull request.
      * 
      * <p><b>200</b> - Successful Response
      * <p><b>422</b> - Validation Error
@@ -53,7 +53,7 @@ public DetectorResponse detectDetectorPost(DetectorRequest detectorRequest) thro
     }
 
     /**
-     * Detect bad practices given rules.
+     * Detect bad practices for given pull request.
      * 
      * <p><b>200</b> - Successful Response
      * <p><b>422</b> - Validation Error

server/application-server/src/main/java/de/tum/in/www1/hephaestus/intelligenceservice/model/BadPractice.java

@@ -29,13 +29,17 @@
  */
 @JsonPropertyOrder({
   BadPractice.JSON_PROPERTY_DESCRIPTION,
+  BadPractice.JSON_PROPERTY_RESOLVED,
   BadPractice.JSON_PROPERTY_TITLE
 })
 @jakarta.annotation.Generated(value = "org.openapitools.codegen.languages.JavaClientCodegen", comments = "Generator version: 7.7.0")
 public class BadPractice {
   public static final String JSON_PROPERTY_DESCRIPTION = "description";
   private String description;
 
+  public static final String JSON_PROPERTY_RESOLVED = "resolved";
+  private Boolean resolved;
+
   public static final String JSON_PROPERTY_TITLE = "title";
   private String title;
 
@@ -67,6 +71,31 @@ public void setDescription(String description) {
     this.description = description;
   }
 
+  public BadPractice resolved(Boolean resolved) {
+
+    this.resolved = resolved;
+    return this;
+  }
+
+  /**
+   * Whether the bad practice has been resolved.
+   * @return resolved
+   */
+  @jakarta.annotation.Nonnull
+  @JsonProperty(JSON_PROPERTY_RESOLVED)
+  @JsonInclude(value = JsonInclude.Include.ALWAYS)
+
+  public Boolean getResolved() {
+    return resolved;
+  }
+
+
+  @JsonProperty(JSON_PROPERTY_RESOLVED)
+  @JsonInclude(value = JsonInclude.Include.ALWAYS)
+  public void setResolved(Boolean resolved) {
+    this.resolved = resolved;
+  }
+
   public BadPractice title(String title) {
 
     this.title = title;
@@ -102,19 +131,21 @@ public boolean equals(Object o) {
     }
     BadPractice badPractice = (BadPractice) o;
     return Objects.equals(this.description, badPractice.description) &&
+        Objects.equals(this.resolved, badPractice.resolved) &&
         Objects.equals(this.title, badPractice.title);
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(description, title);
+    return Objects.hash(description, resolved, title);
   }
 
   @Override
   public String toString() {
     StringBuilder sb = new StringBuilder();
     sb.append("class BadPractice {\n");
     sb.append("    description: ").append(toIndentedString(description)).append("\n");
+    sb.append("    resolved: ").append(toIndentedString(resolved)).append("\n");
     sb.append("    title: ").append(toIndentedString(title)).append("\n");
     sb.append("}");
     return sb.toString();