Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Programming exercises: Improve preliminary AI feedback #9324

Merged
merged 37 commits into from
Oct 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
ab5af3c
move request feedback to a standalone component and add a button to t…
dmytropolityka Sep 17, 2024
9711dcc
set the number of feedback requests to 20
dmytropolityka Sep 17, 2024
1f949c2
change animation and fix error messages
dmytropolityka Sep 17, 2024
02987d0
remove individual due date
dmytropolityka Sep 17, 2024
7ada9fe
Merge branch 'develop' into feature/programming-exercises/revised-fee…
dmytropolityka Sep 17, 2024
d0f9539
use async call chain to request feedback
dmytropolityka Sep 17, 2024
3fa94a6
use async call chain to request feedback
dmytropolityka Sep 17, 2024
1ba71f3
Merge remote-tracking branch 'origin/feature/programming-exercises/re…
dmytropolityka Sep 17, 2024
fe7d0ec
generate rated results
dmytropolityka Sep 17, 2024
65cd345
Merge branch 'develop' into feature/programming-exercises/revised-fee…
dmytropolityka Sep 30, 2024
3e2a994
Merge branch 'develop' into feature/programming-exercises/request-fee…
dmytropolityka Sep 30, 2024
6493b0e
rework notifications
dmytropolityka Sep 30, 2024
42f6b39
implement tests, change subscription type
dmytropolityka Sep 30, 2024
d8d0c9b
cleanup
dmytropolityka Sep 30, 2024
7bd2101
fix test
dmytropolityka Sep 30, 2024
653816d
fix another test
dmytropolityka Sep 30, 2024
dabd872
fix another test
dmytropolityka Sep 30, 2024
02aee43
Merge remote-tracking branch 'origin/feature/programming-exercises/re…
dmytropolityka Oct 1, 2024
41fc7c3
fix missing status icons
dmytropolityka Oct 1, 2024
1f8517b
fix results view for the dashboard
dmytropolityka Oct 2, 2024
babb3ae
reformat
dmytropolityka Oct 2, 2024
adad369
adjust tests
dmytropolityka Oct 2, 2024
21a6d88
adjust server tests
dmytropolityka Oct 2, 2024
da44202
adjust server tests
dmytropolityka Oct 2, 2024
8ba29dd
add value for the number of feedback requests
dmytropolityka Oct 2, 2024
9f8bbca
Merge branch 'develop' into feature/programming-exercises/revised-fee…
dmytropolityka Oct 2, 2024
5b9ec97
Update src/main/webapp/app/exercises/programming/shared/code-editor/a…
dmytropolityka Oct 2, 2024
f83e5d0
Update src/main/webapp/i18n/en/result.json
dmytropolityka Oct 2, 2024
bf32bc6
Update src/main/webapp/i18n/de/result.json
dmytropolityka Oct 2, 2024
f4a2449
comments of Julian
dmytropolityka Oct 3, 2024
82ac204
fix test
dmytropolityka Oct 3, 2024
9a4fec5
improve client style
dmytropolityka Oct 3, 2024
2587bf5
CR of Julian
dmytropolityka Oct 4, 2024
2582aab
set athenaEnabled for modeling exercises
dmytropolityka Oct 5, 2024
129ae1d
Merge branch 'develop' into feature/programming-exercises/revised-fee…
dmytropolityka Oct 7, 2024
eff2cc5
Merge branch 'develop' into feature/programming-exercises/revised-fee…
dmytropolityka Oct 10, 2024
e7865e5
Merge branch 'develop' into feature/programming-exercises/revised-fee…
FelixTJDietrich Oct 11, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -629,7 +629,7 @@ public boolean isAutomatic() {
* @return true if the result is an automatic AI Athena result
*/
@JsonIgnore
public boolean isAthenaAutomatic() {
public boolean isAthenaBased() {
return AssessmentType.AUTOMATIC_ATHENA == assessmentType;
}

dmytropolityka marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -562,8 +562,9 @@ public Submission findLatestSubmissionWithRatedResultWithCompletionDate(Particip
boolean ratedOrPractice = Boolean.TRUE.equals(result.isRated()) || participation.isPracticeMode();
boolean noProgrammingAndAssessmentOver = !isProgrammingExercise && isAssessmentOver;
// For programming exercises we check that the assessment due date has passed (if set) for manual results otherwise we always show the automatic result
boolean programmingAfterAssessmentOrAutomatic = isProgrammingExercise && ((result.isManual() && isAssessmentOver) || result.isAutomatic());
if (ratedOrPractice && (noProgrammingAndAssessmentOver || programmingAfterAssessmentOrAutomatic)) {
boolean programmingAfterAssessmentOrAutomaticOrAthena = isProgrammingExercise
&& ((result.isManual() && isAssessmentOver) || result.isAutomatic() || result.isAthenaBased());
if (ratedOrPractice && (noProgrammingAndAssessmentOver || programmingAfterAssessmentOrAutomaticOrAthena)) {
// take the first found result that fulfills the above requirements
// or
// take newer results and thus disregard older ones
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ public Result getResultForCorrectionRound(int correctionRound) {
*/
@NotNull
private List<Result> filterNonAutomaticResults() {
return results.stream().filter(result -> result == null || !(result.isAutomatic() || result.isAthenaAutomatic())).toList();
return results.stream().filter(result -> result == null || !(result.isAutomatic() || result.isAthenaBased())).toList();
dmytropolityka marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand All @@ -188,8 +188,7 @@ public boolean hasResultForCorrectionRound(int correctionRound) {
*/
@JsonIgnore
public void removeAutomaticResults() {
this.results = this.results.stream().filter(result -> result == null || !(result.isAutomatic() || result.isAthenaAutomatic()))
.collect(Collectors.toCollection(ArrayList::new));
this.results = this.results.stream().filter(result -> result == null || !(result.isAutomatic() || result.isAthenaBased())).collect(Collectors.toCollection(ArrayList::new));
dmytropolityka marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand All @@ -214,7 +213,7 @@ public List<Result> getResults() {

@JsonIgnore
public List<Result> getManualResults() {
return results.stream().filter(result -> result != null && !result.isAutomatic() && !result.isAthenaAutomatic()).collect(Collectors.toCollection(ArrayList::new));
return results.stream().filter(result -> result != null && !result.isAutomatic() && !result.isAthenaBased()).collect(Collectors.toCollection(ArrayList::new));
dmytropolityka marked this conversation as resolved.
Show resolved Hide resolved
}

/**
Expand All @@ -224,7 +223,7 @@ public List<Result> getManualResults() {
*/
@JsonIgnore
public List<Result> getNonAthenaResults() {
return results.stream().filter(result -> result != null && !result.isAthenaAutomatic()).collect(Collectors.toCollection(ArrayList::new));
return results.stream().filter(result -> result != null && !result.isAthenaBased()).collect(Collectors.toCollection(ArrayList::new));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
import jakarta.annotation.Nullable;
import jakarta.validation.constraints.NotNull;

import org.apache.velocity.exception.ResourceNotFoundException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Value;
Expand Down Expand Up @@ -382,7 +381,7 @@ private ResponseEntity<StudentParticipation> handleExerciseFeedbackRequest(Exerc
throw new BadRequestAlertException("Not intended for the use in exams", "participation", "preconditions not met");
}
if (exercise.getDueDate() != null && now().isAfter(exercise.getDueDate())) {
throw new BadRequestAlertException("The due date is over", "participation", "preconditions not met");
throw new BadRequestAlertException("The due date is over", "participation", "feedbackRequestAfterDueDate", true);
dmytropolityka marked this conversation as resolved.
Show resolved Hide resolved
}
if (exercise instanceof ProgrammingExercise) {
((ProgrammingExercise) exercise).validateSettingsForFeedbackRequest();
Expand All @@ -393,7 +392,7 @@ private ResponseEntity<StudentParticipation> handleExerciseFeedbackRequest(Exerc
StudentParticipation participation = (exercise instanceof ProgrammingExercise)
? programmingExerciseParticipationService.findStudentParticipationByExerciseAndStudentId(exercise, principal.getName())
: studentParticipationRepository.findByExerciseIdAndStudentLogin(exercise.getId(), principal.getName())
.orElseThrow(() -> new ResourceNotFoundException("Participation not found"));
.orElseThrow(() -> new BadRequestAlertException("Submission not found", "participation", "noSubmissionExists", true));
dmytropolityka marked this conversation as resolved.
Show resolved Hide resolved

checkAccessPermissionOwner(participation, user);
participation = studentParticipationRepository.findByIdWithResultsElseThrow(participation.getId());
Expand All @@ -406,15 +405,14 @@ private ResponseEntity<StudentParticipation> handleExerciseFeedbackRequest(Exerc
}
else if (exercise instanceof ProgrammingExercise) {
if (participation.findLatestLegalResult() == null) {
throw new BadRequestAlertException("User has not reached the conditions to submit a feedback request", "participation", "preconditions not met");
throw new BadRequestAlertException("You need to submit at least once and have the build results", "participation", "noSubmissionExists", true);
dmytropolityka marked this conversation as resolved.
Show resolved Hide resolved
}
}

// Check if feedback has already been requested
var currentDate = now();
var participationIndividualDueDate = participation.getIndividualDueDate();
if (participationIndividualDueDate != null && currentDate.isAfter(participationIndividualDueDate)) {
throw new BadRequestAlertException("Request has already been sent", "participation", "already sent");
var latestResult = participation.findLatestResult();
if (latestResult != null && latestResult.getAssessmentType() == AssessmentType.AUTOMATIC_ATHENA && latestResult.getCompletionDate().isAfter(now())) {
throw new BadRequestAlertException("Request has already been sent", "participation", "feedbackRequestAlreadySent", true);
dmytropolityka marked this conversation as resolved.
Show resolved Hide resolved
}

// Process feedback request
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -712,8 +712,7 @@ private boolean checkForRatedAndAssessedResult(Result result) {
* @return true if the result is manual and the assessment is over, or it is an automatic result, false otherwise
*/
private boolean checkForAssessedResult(Result result) {
return result.getCompletionDate() != null
&& ((result.isManual() && ExerciseDateService.isAfterAssessmentDueDate(this)) || result.isAutomatic() || result.isAthenaAutomatic());
return result.getCompletionDate() != null && ((result.isManual() && ExerciseDateService.isAfterAssessmentDueDate(this)) || result.isAutomatic() || result.isAthenaBased());
dmytropolityka marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,15 @@
import static java.time.ZonedDateTime.now;

import java.time.ZonedDateTime;
import java.util.Comparator;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.CompletableFuture;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Value;
import org.springframework.context.annotation.Profile;
import org.springframework.stereotype.Service;

Expand Down Expand Up @@ -59,6 +61,9 @@ public class ProgrammingExerciseCodeReviewFeedbackService {

private final ProgrammingMessagingService programmingMessagingService;

@Value("${artemis.athena.allowed-feedback-attempts:20}")
private int allowedFeedbackAttempts;
dmytropolityka marked this conversation as resolved.
Show resolved Hide resolved
dmytropolityka marked this conversation as resolved.
Show resolved Hide resolved

public ProgrammingExerciseCodeReviewFeedbackService(GroupNotificationService groupNotificationService,
Optional<AthenaFeedbackSuggestionsService> athenaFeedbackSuggestionsService, SubmissionService submissionService, ResultService resultService,
ProgrammingExerciseStudentParticipationRepository programmingExerciseStudentParticipationRepository, ResultRepository resultRepository,
Expand Down Expand Up @@ -111,14 +116,14 @@ public void generateAutomaticNonGradedFeedback(ProgrammingExerciseStudentPartici
var submissionOptional = programmingExerciseParticipationService.findProgrammingExerciseParticipationWithLatestSubmissionAndResult(participation.getId())
.findLatestSubmission();
if (submissionOptional.isEmpty()) {
throw new BadRequestAlertException("No legal submissions found", "submission", "noSubmission");
throw new BadRequestAlertException("No legal submissions found", "submission", "noSubmissionExists");
}
var submission = submissionOptional.get();

// save result and transmit it over websockets to notify the client about the status
var automaticResult = this.submissionService.saveNewEmptyResult(submission);
automaticResult.setAssessmentType(AssessmentType.AUTOMATIC_ATHENA);
automaticResult.setRated(false);
automaticResult.setRated(true); // we want to use this feedback to give the grade in the future
dmytropolityka marked this conversation as resolved.
Show resolved Hide resolved
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
Expand All @@ -127,7 +132,6 @@ public void generateAutomaticNonGradedFeedback(ProgrammingExerciseStudentPartici

try {

setIndividualDueDateAndLockRepository(participation, programmingExercise, false);
this.programmingMessagingService.notifyUserAboutNewResult(automaticResult, participation);
// now the client should be able to see new result

Expand Down Expand Up @@ -158,9 +162,10 @@ public void generateAutomaticNonGradedFeedback(ProgrammingExerciseStudentPartici
feedback.setDetailText(individualFeedbackItem.description());
feedback.setHasLongFeedbackText(false);
feedback.setType(FeedbackType.AUTOMATIC);
feedback.setCredits(0.0);
feedback.setCredits(individualFeedbackItem.credits());
return feedback;
}).toList();
}).sorted(Comparator.comparing(Feedback::getCredits, Comparator.nullsLast(Comparator.naturalOrder()))).toList();
;

automaticResult.setSuccessful(true);
automaticResult.setCompletionDate(ZonedDateTime.now());
Expand All @@ -176,9 +181,6 @@ public void generateAutomaticNonGradedFeedback(ProgrammingExerciseStudentPartici
this.resultRepository.save(automaticResult);
this.programmingMessagingService.notifyUserAboutNewResult(automaticResult, participation);
}
finally {
unlockRepository(participation, programmingExercise);
}
}

/**
Expand Down Expand Up @@ -225,15 +227,10 @@ private void checkRateLimitOrThrow(ProgrammingExerciseStudentParticipation parti

List<Result> athenaResults = participation.getResults().stream().filter(result -> result.getAssessmentType() == AssessmentType.AUTOMATIC_ATHENA).toList();

long countOfAthenaResultsInProcessOrSuccessful = athenaResults.stream().filter(result -> result.isSuccessful() == null || result.isSuccessful() == Boolean.TRUE).count();

long countOfSuccessfulRequests = athenaResults.stream().filter(result -> result.isSuccessful() == Boolean.TRUE).count();

if (countOfAthenaResultsInProcessOrSuccessful >= 3) {
throw new BadRequestAlertException("Cannot send additional AI feedback requests now. Try again later!", "participation", "preconditions not met");
}
if (countOfSuccessfulRequests >= 20) {
throw new BadRequestAlertException("Maximum number of AI feedback requests reached.", "participation", "preconditions not met");
if (countOfSuccessfulRequests >= this.allowedFeedbackAttempts) {
throw new BadRequestAlertException("Maximum number of AI feedback requests reached.", "participation", "maxAthenaResultsReached", true);
dmytropolityka marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
18 changes: 0 additions & 18 deletions src/main/webapp/app/entities/result.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,24 +39,6 @@ export class Result implements BaseEntity {
this.successful = false; // default value
}

/**
* Checks whether the result is a manual result. A manual result can be from type MANUAL or SEMI_AUTOMATIC
*
* @return true if the result is a manual result
*/
public static isManualResult(that: Result): boolean {
return that.assessmentType === AssessmentType.MANUAL || that.assessmentType === AssessmentType.SEMI_AUTOMATIC;
}

/**
* Checks whether the result is generated by Athena AI.
*
* @return true if the result is an automatic Athena AI result
*/
public static isAthenaAIResult(that: Result): boolean {
return that.assessmentType === AssessmentType.AUTOMATIC_ATHENA;
}

/**
* Checks whether the given result has an assessment note that is not empty.
* @param that the result of which the presence of an assessment note is being checked
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { ExerciseHint } from 'app/entities/hestia/exercise-hint.model';
import { ExerciseHintService } from 'app/exercises/shared/exercise-hint/shared/exercise-hint.service';
import { HttpResponse } from '@angular/common/http';
import { AlertService } from 'app/core/util/alert.service';
import { isManualResult as isManualResultFunction } from 'app/exercises/shared/result/result.utils';

@Component({
selector: 'jhi-code-editor-student',
Expand Down Expand Up @@ -148,7 +149,7 @@ export class CodeEditorStudentContainerComponent implements OnInit, OnDestroy {
let hasTutorFeedback = false;
if (this.latestResult) {
// latest result is the first element of results, see loadParticipationWithLatestResult
isManualResult = Result.isManualResult(this.latestResult);
isManualResult = isManualResultFunction(this.latestResult);
if (isManualResult) {
hasTutorFeedback = this.latestResult.feedbacks!.some((feedback) => feedback.type === FeedbackType.MANUAL);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { SubmissionType } from 'app/entities/submission.model';
import { ProgrammingExercise } from 'app/entities/programming/programming-exercise.model';
import { AlertService } from 'app/core/util/alert.service';
import { hasParticipationChanged } from 'app/exercises/shared/participation/participation.utils';
import { isManualResult } from 'app/exercises/shared/result/result.utils';

/**
* Component for triggering a build for the CURRENT submission of the student (does not create a new commit!).
Expand Down Expand Up @@ -60,7 +61,7 @@ export abstract class ProgrammingExerciseTriggerBuildButtonComponent implements
if (hasDueDatePassed(this.exercise)) {
// If the last result was manual, the instructor might not want to override it with a new automatic result.
const newestResult = !!this.participation.results && head(orderBy(this.participation.results, ['id'], ['desc']));
this.lastResultIsManual = !!newestResult && Result.isManualResult(newestResult);
this.lastResultIsManual = !!newestResult && isManualResult(newestResult);
dmytropolityka marked this conversation as resolved.
Show resolved Hide resolved
}
// We can trigger the build only if the participation is active (has build plan), if the build plan was archived (new build plan will be created)
// or the due date is over.
Expand Down Expand Up @@ -126,7 +127,7 @@ export abstract class ProgrammingExerciseTriggerBuildButtonComponent implements
.pipe(
filter((result) => !!result),
tap((result: Result) => {
this.lastResultIsManual = !!result && Result.isManualResult(result);
this.lastResultIsManual = !!result && isManualResult(result);
dmytropolityka marked this conversation as resolved.
Show resolved Hide resolved
}),
)
.subscribe();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
@if (!!participation()?.exercise) {
<jhi-request-feedback-button class="me-3" [exercise]="participation()!.exercise!"></jhi-request-feedback-button>
}
@if (commitState === CommitState.CONFLICT) {
<div>
<button
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Component, EventEmitter, Input, OnChanges, OnDestroy, OnInit, Output, SimpleChanges } from '@angular/core';
import { Component, EventEmitter, Input, OnChanges, OnDestroy, OnInit, Output, SimpleChanges, input } from '@angular/core';
import { HttpErrorResponse } from '@angular/common/http';
import { NgbModal } from '@ng-bootstrap/ng-bootstrap';
import { catchError, switchMap, tap } from 'rxjs/operators';
Expand All @@ -14,6 +14,7 @@ import { CodeEditorConfirmRefreshModalComponent } from './code-editor-confirm-re
import { AUTOSAVE_CHECK_INTERVAL, AUTOSAVE_EXERCISE_INTERVAL } from 'app/shared/constants/exercise-exam-constants';
import { faCircleNotch, faSync, faTimes } from '@fortawesome/free-solid-svg-icons';
import { faPlayCircle } from '@fortawesome/free-regular-svg-icons';
import { Participation } from 'app/entities/participation/participation.model';

@Component({
selector: 'jhi-code-editor-actions',
Expand All @@ -34,6 +35,7 @@ export class CodeEditorActionsComponent implements OnInit, OnDestroy, OnChanges
@Input() get commitState() {
return this.commitStateValue;
}
participation = input<Participation>();

@Output() commitStateChange = new EventEmitter<CommitState>();
@Output() editorStateChange = new EventEmitter<EditorState>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { CodeEditorHeaderComponent } from 'app/exercises/programming/shared/code
import { CodeEditorFileBrowserBadgeComponent } from 'app/exercises/programming/shared/code-editor/file-browser/code-editor-file-browser-badge.component';
import { CodeEditorMonacoComponent } from 'app/exercises/programming/shared/code-editor/monaco/code-editor-monaco.component';
import { ArtemisSharedComponentModule } from 'app/shared/components/shared-component.module';
import { RequestFeedbackButtonComponent } from 'app/overview/exercise-details/request-feedback-button/request-feedback-button.component';
import { MonacoEditorComponent } from 'app/shared/monaco-editor/monaco-editor.component';

@NgModule({
Expand All @@ -35,6 +36,7 @@ import { MonacoEditorComponent } from 'app/shared/monaco-editor/monaco-editor.co
ArtemisProgrammingManualAssessmentModule,
MonacoEditorComponent,
ArtemisSharedComponentModule,
RequestFeedbackButtonComponent,
dmytropolityka marked this conversation as resolved.
Show resolved Hide resolved
],
declarations: [
CodeEditorGridComponent,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ <h4 class="editor-title"><ng-content select="[editorTitle]" /></h4>
(onRefreshFiles)="onRefreshFiles()"
(commitStateChange)="onCommitStateChange.emit($event)"
(onError)="onError($event)"
[participation]="participation"
dmytropolityka marked this conversation as resolved.
Show resolved Hide resolved
dmytropolityka marked this conversation as resolved.
Show resolved Hide resolved
/>
}
</div>
Expand Down
Loading
Loading