Programming exercises: Improve integration of Monaco code editor and Athena Preliminary Feedback

src/main/java/de/tum/cit/aet/artemis/programming/service/ProgrammingExerciseCodeReviewFeedbackService.java

@@ -43,7 +43,8 @@ public class ProgrammingExerciseCodeReviewFeedbackService {
 
     private static final Logger log = LoggerFactory.getLogger(ProgrammingExerciseCodeReviewFeedbackService.class);
 
-    public static final String NON_GRADED_FEEDBACK_SUGGESTION = "NonGradedFeedbackSuggestion:";
+    // feedback.detailText prefix
+    public static final String PRELIMINARY_FEEDBACK_PREFIX = "NonGradedFeedbackSuggestion:";
 
     private final GroupNotificationService groupNotificationService;
 
@@ -124,7 +125,7 @@ public void generateAutomaticNonGradedFeedback(ProgrammingExerciseStudentPartici
         automaticResult.setScore(100.0);
         automaticResult.setSuccessful(null);
         automaticResult.setCompletionDate(ZonedDateTime.now().plusMinutes(5)); // we do not want to show dates without a completion date, but we want the students to know their
-                                                                               // feedback request is in work
+        // feedback request is in work
         automaticResult = this.resultRepository.save(automaticResult);
 
         try {
@@ -143,17 +144,17 @@ public void generateAutomaticNonGradedFeedback(ProgrammingExerciseStudentPartici
                         String feedbackText;
                         if (Objects.nonNull(individualFeedbackItem.lineStart())) {
                             if (Objects.nonNull(individualFeedbackItem.lineEnd()) && !individualFeedbackItem.lineStart().equals(individualFeedbackItem.lineEnd())) {
-                                feedbackText = String.format(NON_GRADED_FEEDBACK_SUGGESTION + "File %s at lines %d-%d", individualFeedbackItem.filePath(),
-                                        individualFeedbackItem.lineStart(), individualFeedbackItem.lineEnd());
+                                feedbackText = String.format(PRELIMINARY_FEEDBACK_PREFIX + "File %s at lines %d-%d", individualFeedbackItem.filePath(),
+                                        individualFeedbackItem.lineStart() + 1, individualFeedbackItem.lineEnd() + 1);
                             }
                             else {
-                                feedbackText = String.format(NON_GRADED_FEEDBACK_SUGGESTION + "File %s at line %d", individualFeedbackItem.filePath(),
-                                        individualFeedbackItem.lineStart());
+                                feedbackText = String.format(PRELIMINARY_FEEDBACK_PREFIX + "File %s at line %d", individualFeedbackItem.filePath(),
+                                        individualFeedbackItem.lineStart() + 1);
                             }
                             feedback.setReference(String.format("file:%s_line:%d", individualFeedbackItem.filePath(), individualFeedbackItem.lineStart()));
                         }
                         else {
-                            feedbackText = String.format(NON_GRADED_FEEDBACK_SUGGESTION + "File %s", individualFeedbackItem.filePath());
+                            feedbackText = String.format(PRELIMINARY_FEEDBACK_PREFIX + "File %s", individualFeedbackItem.filePath());
                         }
                         feedback.setText(feedbackText);
                         feedback.setDetailText(individualFeedbackItem.description());

src/main/webapp/app/assessment/shared/entities/feedback.model.spec.ts

@@ -2,7 +2,7 @@ import {
     Feedback,
     FeedbackSuggestionType,
     FeedbackType,
-    NON_GRADED_FEEDBACK_SUGGESTION_IDENTIFIER,
+    PRELIMINARY_FEEDBACK_IDENTIFIER,
     STATIC_CODE_ANALYSIS_FEEDBACK_IDENTIFIER,
     SUBMISSION_POLICY_FEEDBACK_IDENTIFIER,
     buildFeedbackTextForReview,
@@ -162,8 +162,37 @@ describe('Feedback', () => {
         });
 
         it('should correctly detect non graded automatically generated feedback', () => {
-            const feedback: Feedback = { type: FeedbackType.AUTOMATIC, text: NON_GRADED_FEEDBACK_SUGGESTION_IDENTIFIER + 'content' };
-            expect(Feedback.isNonGradedFeedbackSuggestion(feedback)).toBeTrue();
+            const feedback: Feedback = { type: FeedbackType.AUTOMATIC, text: PRELIMINARY_FEEDBACK_IDENTIFIER + 'content' };
+            expect(Feedback.isPreliminaryFeedback(feedback)).toBeTrue();
+        });
+    });
+
+    describe('Feedback with lines information', () => {
+        it('should include lines information in feedback text if text contains "lines"', () => {
+            const feedback = new Feedback();
+            feedback.detailText = 'This is a detailed feedback.';
+            feedback.text = 'NonGradedFeedbackSuggestion:File src/example/file.java at lines 10-20';
+
+            const expectedText = 'This is a detailed feedback. (lines 10-20)';
+            expect(buildFeedbackTextForReview(feedback)).toBe(expectedText);
+        });
+
+        it('should not include lines information if text does not contain "lines"', () => {
+            const feedback = new Feedback();
+            feedback.detailText = 'This is a detailed feedback.';
+            feedback.text = 'NonGradedFeedbackSuggestion:File src/example/file.java';
+
+            const expectedText = 'This is a detailed feedback.';
+            expect(buildFeedbackTextForReview(feedback)).toBe(expectedText);
+        });
+
+        it('should ignore feedback not starting with the expected prefix', () => {
+            const feedback = new Feedback();
+            feedback.detailText = 'This is another type of feedback.';
+            feedback.text = 'Some other feedback text unrelated to lines';
+
+            const expectedText = 'This is another type of feedback.';
+            expect(buildFeedbackTextForReview(feedback)).toBe(expectedText);
         });
     });
 });

src/main/webapp/app/assessment/shared/entities/feedback.model.ts

@@ -33,7 +33,7 @@ export const SUBMISSION_POLICY_FEEDBACK_IDENTIFIER = 'SubPolFeedbackIdentifier:'
 export const FEEDBACK_SUGGESTION_IDENTIFIER = 'FeedbackSuggestion:';
 export const FEEDBACK_SUGGESTION_ACCEPTED_IDENTIFIER = 'FeedbackSuggestion:accepted:';
 export const FEEDBACK_SUGGESTION_ADAPTED_IDENTIFIER = 'FeedbackSuggestion:adapted:';
-export const NON_GRADED_FEEDBACK_SUGGESTION_IDENTIFIER = 'NonGradedFeedbackSuggestion:';
+export const PRELIMINARY_FEEDBACK_IDENTIFIER = 'NonGradedFeedbackSuggestion:';
 
 export interface DropInfo {
     instruction: GradingInstruction;
@@ -123,11 +123,11 @@ export class Feedback implements BaseEntity {
         return that.text.startsWith(FEEDBACK_SUGGESTION_IDENTIFIER);
     }
 
-    public static isNonGradedFeedbackSuggestion(that: Feedback): boolean {
+    public static isPreliminaryFeedback(that: Feedback): boolean {
         if (!that.text) {
             return false;
         }
-        return that.text.startsWith(NON_GRADED_FEEDBACK_SUGGESTION_IDENTIFIER);
+        return that.text.startsWith(PRELIMINARY_FEEDBACK_IDENTIFIER);
     }
 
     /**
@@ -297,6 +297,12 @@ export const buildFeedbackTextForReview = (feedback: Feedback, addFeedbackText =
         }
     } else if (feedback.detailText) {
         feedbackText = feedback.detailText;
+        if (feedback.text && feedback.text.includes('lines')) {
+            const linesInfo = feedback.text.match(/lines .*/);
+            if (linesInfo) {
+                feedbackText += ` (${linesInfo[0]})`;
+            }
+        }
     } else if (addFeedbackText && feedback.text) {
         feedbackText = feedback.text;
     }

src/main/webapp/app/exercise/feedback/item/programming-feedback-item.service.ts

@@ -6,7 +6,7 @@ import {
     FEEDBACK_SUGGESTION_IDENTIFIER,
     Feedback,
     FeedbackType,
-    NON_GRADED_FEEDBACK_SUGGESTION_IDENTIFIER,
+    PRELIMINARY_FEEDBACK_IDENTIFIER,
     STATIC_CODE_ANALYSIS_FEEDBACK_IDENTIFIER,
     SUBMISSION_POLICY_FEEDBACK_IDENTIFIER,
 } from 'app/assessment/shared/entities/feedback.model';
@@ -50,9 +50,9 @@ export class ProgrammingFeedbackItemService implements FeedbackItemService {
             return this.createScaFeedbackItem(feedback, showTestDetails);
         } else if (Feedback.isFeedbackSuggestion(feedback)) {
             return this.createFeedbackSuggestionItem(feedback, showTestDetails);
-        } else if (feedback.type === FeedbackType.AUTOMATIC && !Feedback.isNonGradedFeedbackSuggestion(feedback)) {
+        } else if (feedback.type === FeedbackType.AUTOMATIC && !Feedback.isPreliminaryFeedback(feedback)) {
             return this.createAutomaticFeedbackItem(feedback, showTestDetails);
-        } else if (feedback.type === FeedbackType.AUTOMATIC && Feedback.isNonGradedFeedbackSuggestion(feedback)) {
+        } else if (feedback.type === FeedbackType.AUTOMATIC && Feedback.isPreliminaryFeedback(feedback)) {
             return this.createNonGradedFeedbackItem(feedback);
         } else if ((feedback.type === FeedbackType.MANUAL || feedback.type === FeedbackType.MANUAL_UNREFERENCED) && feedback.gradingInstruction) {
             return this.createGradingInstructionFeedbackItem(feedback, showTestDetails);
@@ -161,7 +161,7 @@ export class ProgrammingFeedbackItemService implements FeedbackItemService {
         return {
             type: 'Reviewer',
             name: this.translateService.instant('artemisApp.result.detail.feedback'),
-            title: feedback.text?.slice(NON_GRADED_FEEDBACK_SUGGESTION_IDENTIFIER.length),
+            title: feedback.text?.slice(PRELIMINARY_FEEDBACK_IDENTIFIER.length),
             text: feedback.detailText,
             positive: feedback.positive,
             credits: feedback.credits,

src/main/webapp/app/exercise/result/result.service.spec.ts

@@ -22,7 +22,7 @@ import { AssessmentType } from 'app/assessment/shared/entities/assessment-type.m
 import { ProgrammingSubmission } from 'app/programming/shared/entities/programming-submission.model';
 import {
     FeedbackType,
-    NON_GRADED_FEEDBACK_SUGGESTION_IDENTIFIER,
+    PRELIMINARY_FEEDBACK_IDENTIFIER,
     STATIC_CODE_ANALYSIS_FEEDBACK_IDENTIFIER,
     SUBMISSION_POLICY_FEEDBACK_IDENTIFIER,
 } from 'app/assessment/shared/entities/feedback.model';
@@ -90,14 +90,14 @@ describe('ResultService', () => {
         score: 80,
     };
     const result6: Result = {
-        feedbacks: [{ text: NON_GRADED_FEEDBACK_SUGGESTION_IDENTIFIER + 'AI feedback', type: FeedbackType.AUTOMATIC }],
+        feedbacks: [{ text: PRELIMINARY_FEEDBACK_IDENTIFIER + 'AI feedback', type: FeedbackType.AUTOMATIC }],
         participation: { type: ParticipationType.PROGRAMMING },
         completionDate: dayjs().subtract(5, 'minutes'),
         assessmentType: AssessmentType.AUTOMATIC_ATHENA,
         successful: true,
     };
     const result7: Result = {
-        feedbacks: [{ text: NON_GRADED_FEEDBACK_SUGGESTION_IDENTIFIER + 'AI feedback', type: FeedbackType.AUTOMATIC }],
+        feedbacks: [{ text: PRELIMINARY_FEEDBACK_IDENTIFIER + 'AI feedback', type: FeedbackType.AUTOMATIC }],
         participation: { type: ParticipationType.PROGRAMMING },
         completionDate: dayjs().subtract(5, 'minutes'),
         assessmentType: AssessmentType.AUTOMATIC_ATHENA,

src/main/webapp/app/exercise/result/updating-result/updating-result.component.ts

@@ -42,7 +42,7 @@ export class UpdatingResultComponent implements OnChanges, OnDestroy {
     @Input() showCompletion = true;
     @Input() showProgressBar = false;
     @Input() showProgressBarBorder = false;
-    @Output() showResult = new EventEmitter<void>();
+    @Output() showResult = new EventEmitter<Result>();
     /**
      * @property personalParticipation Whether the participation belongs to the user (by being a student) or not (by being an instructor)
      */
@@ -80,7 +80,7 @@ export class UpdatingResultComponent implements OnChanges, OnDestroy {
             }
 
             if (this.result) {
-                this.showResult.emit();
+                this.showResult.emit(this.result);
             }
         }
     }
@@ -125,7 +125,7 @@ export class UpdatingResultComponent implements OnChanges, OnDestroy {
                     }
                     this.onParticipationChange.emit();
                     if (result) {
-                        this.showResult.emit();
+                        this.showResult.emit(this.result);
                     }
                 }),
             )

src/main/webapp/app/programming/manage/assess/code-editor-tutor-assessment-inline-feedback.component.scss

@@ -0,0 +1,14 @@
+.cross {
+    width: 20px;
+    height: 20px;
+    padding: 2px;
+    display: flex;
+    align-items: center;
+    justify-content: center;
+    border: 1px solid #000;
+
+    span {
+        font-size: 10px;
+        line-height: 1;
+    }
+}

src/main/webapp/app/programming/manage/assess/code-editor-tutor-assessment-inline-feedback/code-editor-tutor-assessment-inline-feedback.component.html

@@ -71,7 +71,7 @@
                         [translateValues]="{ text: this.feedback | feedbackContent | quoted: ' ' }"
                         deleteQuestion="artemisApp.feedback.delete.question"
                         type="submit"
-                        (delete)="deleteFeedback()"
+                        (delete)="deleteFeedback(false)"
                         [dialogError]="dialogError$"
                         class="btn btn-danger btn-sm me-1"
                     >
@@ -90,24 +90,31 @@
             }
         </div>
     } @else {
-        <div class="p-1">
+        <div class="p-1 position-relative">
+            @if (Feedback.isPreliminaryFeedback(feedback)) {
+                <button
+                    type="button"
+                    class="btn-close cross position-absolute top-0 end-0 m-1 rounded-circle"
+                    aria-label="Close"
+                    (click)="deleteFeedback(true)"
+                    [ngbTooltip]="'entity.action.close' | artemisTranslate"
+                ></button>
+            }
             <div class="row flex-nowrap align-items-top m-1">
                 <div class="col flex-grow-0 ps-0">
                     <h5 class="d-inline">
-                        @if (!Feedback.isNonGradedFeedbackSuggestion(feedback)) {
-                            <span
-                                class="badge"
-                                [class.bg-success]="feedback.credits! > 0 && feedback.isSubsequent === undefined"
-                                [class.bg-danger]="feedback.credits! < 0 && feedback.isSubsequent === undefined"
-                                [class.bg-warning]="feedback.credits === 0 && feedback.isSubsequent === undefined"
-                                [class.bg-secondary]="readOnly && feedback.isSubsequent"
-                                >{{ roundScoreSpecifiedByCourseSettings(feedback.credits, course) + 'P' }}</span
-                            >
-                        }
+                        <span
+                            class="badge"
+                            [class.bg-success]="feedback.credits! > 0 && feedback.isSubsequent === undefined"
+                            [class.bg-danger]="feedback.credits! < 0 && feedback.isSubsequent === undefined"
+                            [class.bg-warning]="feedback.credits === 0 && feedback.isSubsequent === undefined"
+                            [class.bg-secondary]="readOnly && feedback.isSubsequent"
+                            >{{ roundScoreSpecifiedByCourseSettings(feedback.credits, course) + 'P' }}</span
+                        >
                     </h5>
                 </div>
                 <div class="col-10 ps-0 flex-grow-1 flex-shrink-1">
-                    @if (Feedback.isNonGradedFeedbackSuggestion(feedback)) {
+                    @if (Feedback.isPreliminaryFeedback(feedback)) {
                         <h6 class="d-inline mb-1" jhiTranslate="artemisApp.assessment.detail.feedback"></h6>
                         <p [innerHTML]="buildFeedbackTextForCodeEditor(feedback)" class="mt-1 mb-0"></p>
                     } @else {