diff --git a/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/Assessment.java b/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/Assessment.java
index 798acb4..8c5f0a6 100644
--- a/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/Assessment.java
+++ b/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/Assessment.java
@@ -3,11 +3,15 @@
import java.text.MessageFormat;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
+import java.util.Comparator;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
+import java.util.function.Predicate;
import java.util.stream.Collectors;
import edu.kit.kastel.sdq.artemis4j.ArtemisNetworkException;
@@ -16,7 +20,9 @@
import edu.kit.kastel.sdq.artemis4j.client.FeedbackType;
import edu.kit.kastel.sdq.artemis4j.client.ProgrammingSubmissionDTO;
import edu.kit.kastel.sdq.artemis4j.client.ResultDTO;
+import edu.kit.kastel.sdq.artemis4j.grading.location.ComparatorUtils;
import edu.kit.kastel.sdq.artemis4j.grading.location.Location;
+import edu.kit.kastel.sdq.artemis4j.grading.location.LocationFormatter;
import edu.kit.kastel.sdq.artemis4j.grading.metajson.AnnotationMappingException;
import edu.kit.kastel.sdq.artemis4j.grading.metajson.MetaFeedbackMapper;
import edu.kit.kastel.sdq.artemis4j.grading.penalty.GradingConfig;
@@ -48,13 +54,29 @@ public class Assessment extends ArtemisConnectionHolder {
private static final FormatString GLOBAL_FEEDBACK_MISTAKE_TYPE_HEADER_NONSCORING =
new FormatString(new MessageFormat(" * {0}:"));
private static final FormatString GLOBAL_FEEDBACK_ANNOTATION =
- new FormatString(new MessageFormat(" * {0} " + "at line {1,number}"));
+ new FormatString(new MessageFormat(" * {0} at line {1}"));
+ private static final FormatString GLOBAL_FEEDBACK_ANNOTATION_MULTIPLE =
+ new FormatString(new MessageFormat(" * {0} at lines {1}"));
private static final FormatString GLOBAL_FEEDBACK_ANNOTATION_CUSTOM_PENALTY =
- new FormatString(new MessageFormat(" * {0} at line {1,number} ({2,number,##.###}P)"));
+ new FormatString(new MessageFormat(" * {0} at line {1} ({2,number,##.###}P)"));
+ private static final FormatString GLOBAL_FEEDBACK_ANNOTATION_CUSTOM_PENALTY_MULTIPLE =
+ new FormatString(new MessageFormat(" * {0} at lines {1} ({2,number,##.###}P)"));
private static final FormatString GLOBAL_FEEDBACK_LIMIT_OVERRUN = new FormatString(
new MessageFormat(" * Note:" + " The sum of penalties hit the limits for this rating group."));
private static final FormatString NO_FEEDBACK_DUMMY = new FormatString("The tutor has made no annotations.");
+ /**
+ * The global feedback containing a list of all annotations that were made, might be too large. In this case,
+ * the feedback is split into multiple parts. Artemis will display feedbacks that subtract points in a different
+ * section than feedbacks that do not subtract points.
+ *
+ * With only the first feedback deducting points, the rest would be displayed in the info section. To prevent this,
+ * the other parts are passed a negative score that is as close to zero as possible.
+ *
+ * This negative score is defined by the following constant.
+ */
+ private static final double GLOBAL_FEEDBACK_OTHER_PARTS_SCORE = -Double.MIN_VALUE;
+
private final ResultDTO lockingResult;
private final List annotations;
private final List testResults;
@@ -123,7 +145,11 @@ public List getAnnotations() {
* @return An unmodifiable list of annotations, possibly empty but never null.
*/
public List getAnnotations(MistakeType mistakeType) {
- return this.annotations.stream()
+ return this.getAnnotations(mistakeType, this.annotations);
+ }
+
+ private List getAnnotations(MistakeType mistakeType, Collection annotations) {
+ return annotations.stream()
.filter(a -> a.getMistakeType().equals(mistakeType))
.toList();
}
@@ -397,13 +423,8 @@ private void internalSaveOrSubmit(boolean shouldSubmit) throws AnnotationMapping
this.programmingSubmission.getId(), relativeScore, feedbacks, this.lockingResult);
// Sanity check
- double feedbackPoints = Math.min(
- Math.max(
- result.feedbacks().stream()
- .mapToDouble(FeedbackDTO::credits)
- .sum(),
- 0.0),
- this.getMaxPoints());
+ double feedbackPoints = Math.clamp(
+ result.feedbacks().stream().mapToDouble(FeedbackDTO::credits).sum(), 0.0, this.getMaxPoints());
if (Math.abs(absoluteScore - feedbackPoints) > 1e-7) {
throw new IllegalStateException("Feedback points do not match the calculated points. Calculated "
+ absoluteScore + " but feedbacks sum up to " + feedbackPoints + " points.");
@@ -442,7 +463,7 @@ private List packAssessmentForArtemis() throws AnnotationMappingExc
.map(this::createInlineFeedback)
.toList());
- // We have on (or more if they are too long) global feedback per rating group
+ // We have one (or more if they are too long) global feedback per rating group
// These feedbacks deduct points
feedbacks.addAll(this.config.getRatingGroups().stream()
.flatMap(r -> this.createGlobalFeedback(r).stream())
@@ -472,8 +493,8 @@ private List packAssessmentForArtemis() throws AnnotationMappingExc
return feedbacks;
}
- private FeedbackDTO createInlineFeedback(Map.Entry> annotations) {
- var sampleAnnotation = annotations.getValue().get(0);
+ private FeedbackDTO createInlineFeedback(Map.Entry> annotations) {
+ var sampleAnnotation = annotations.getValue().getFirst();
String text =
"File " + sampleAnnotation.getFilePathWithoutType() + " at line " + sampleAnnotation.getDisplayLine();
@@ -511,6 +532,40 @@ private FeedbackDTO createInlineFeedback(Map.Entry> an
return FeedbackDTO.newManual(0.0, text, reference, detailText);
}
+ private TranslatableString formatGlobalFeedbackAnnotations(List annotations, boolean hasScore) {
+ LocationFormatter formatter = new LocationFormatter()
+ // .enableLineMerging()
+ .removeSharedPrefix(true)
+ // only show the starting line number
+ .setLocationToString(location -> "" + (location.start().line() + 1));
+
+ for (var annotation : annotations) {
+ formatter.addLocation(annotation.getLocation());
+ }
+
+ String filePath = annotations.getFirst().getFilePath();
+
+ if (hasScore) {
+ double customScore = annotations.stream()
+ .mapToDouble(a -> a.getCustomScore().get())
+ .sum();
+
+ FormatString formatString = GLOBAL_FEEDBACK_ANNOTATION_CUSTOM_PENALTY_MULTIPLE;
+ if (annotations.size() == 1) {
+ formatString = GLOBAL_FEEDBACK_ANNOTATION_CUSTOM_PENALTY;
+ }
+
+ return formatString.format(filePath, formatter.format(), customScore);
+ } else {
+ FormatString formatString = GLOBAL_FEEDBACK_ANNOTATION_MULTIPLE;
+ if (annotations.size() == 1) {
+ formatString = GLOBAL_FEEDBACK_ANNOTATION;
+ }
+
+ return formatString.format(filePath, formatter.format());
+ }
+ }
+
/**
* This builds one (or more if the feedback is too long) global feedback for a
* rating group. The feedback deducts points, and lists all annotations that are
@@ -525,35 +580,61 @@ private List createGlobalFeedback(RatingGroup ratingGroup) {
var header = GLOBAL_FEEDBACK_HEADER.format(
ratingGroup.getDisplayName(), points.score(), ratingGroup.getMinPenalty(), ratingGroup.getMaxPenalty());
- // First collect only the lines so that we can later split the feedback by lines
- List lines = new ArrayList<>();
+ // group annotations by mistake type
+ List>> annotationsByType = new ArrayList<>();
for (var mistakeType : ratingGroup.getMistakeTypes()) {
+ var annotationsWithType = this.getAnnotations(mistakeType, this.annotations);
+ if (!annotationsWithType.isEmpty()) {
+ annotationsByType.add(Map.entry(mistakeType, annotationsWithType));
+ }
+ }
+
+ // Sort the entries by their size, then by their elements location
+ annotationsByType.sort(Map.Entry.comparingByValue(ComparatorUtils.shortestFirst(
+ ComparatorUtils.compareByElement(Comparator.comparing(Annotation::getLocation)))));
+
+ // First collect only the lines so that we can later split the feedback by lines
+ Collection lines = new ArrayList<>();
+ for (var entry : annotationsByType) {
+ MistakeType mistakeType = entry.getKey();
+ List annotationsWithType = entry.getValue();
+
Optional mistakePoints = this.calculatePointsForMistakeType(mistakeType);
- if (mistakePoints.isPresent()) {
-
- // Header per mistake type
- if (ratingGroup.isScoringGroup() && mistakeType.shouldScore()) {
- lines.add(GLOBAL_FEEDBACK_MISTAKE_TYPE_HEADER.format(
- mistakeType.getButtonText(), mistakePoints.get().score()));
- } else {
- // We don't want to display points if the rating group does not score
- lines.add(GLOBAL_FEEDBACK_MISTAKE_TYPE_HEADER_NONSCORING.format(mistakeType.getButtonText()));
- }
+ if (mistakePoints.isEmpty()) {
+ continue;
+ }
+ // Header per mistake type
+ if (ratingGroup.isScoringGroup() && mistakeType.shouldScore()) {
+ lines.add(GLOBAL_FEEDBACK_MISTAKE_TYPE_HEADER.format(
+ mistakeType.getButtonText(), mistakePoints.get().score()));
+ } else {
+ // We don't want to display points if the rating group does not score
+ lines.add(GLOBAL_FEEDBACK_MISTAKE_TYPE_HEADER_NONSCORING.format(mistakeType.getButtonText()));
+ }
+
+ var annotationsByFilePath = annotationsWithType.stream()
+ .collect(Collectors.groupingBy(Annotation::getFilePath, LinkedHashMap::new, Collectors.toList()));
+
+ for (var annotations : annotationsByFilePath.values()) {
// Individual annotations
- for (var annotation : this.getAnnotations(mistakeType)) {
- // For custom annotations, we have '* at line (P)'
- // Otherwise, it's just '* at line '
- // Lines are zero-indexed
- if (annotation.getCustomScore().isPresent() && ratingGroup.isScoringGroup()) {
- lines.add(GLOBAL_FEEDBACK_ANNOTATION_CUSTOM_PENALTY.format(
- annotation.getFilePath(),
- annotation.getDisplayLine(),
- annotation.getCustomScore().get()));
- } else {
- lines.add(GLOBAL_FEEDBACK_ANNOTATION.format(
- annotation.getFilePath(), annotation.getDisplayLine()));
- }
+ Predicate hasScore = a -> a.getCustomScore().isPresent() && ratingGroup.isScoringGroup();
+
+ // separate annotations with and without score
+ List annotationsWithScore =
+ annotations.stream().filter(hasScore).toList();
+ List annotationsWithoutScore =
+ annotations.stream().filter(Predicate.not(hasScore)).toList();
+
+ // For custom annotations, we have '* at line (P)'
+ if (!annotationsWithScore.isEmpty()) {
+ lines.add(this.formatGlobalFeedbackAnnotations(annotationsWithScore, true));
+ }
+
+ // Otherwise, it's just '* at line '
+ // Lines are zero-indexed
+ if (!annotationsWithoutScore.isEmpty()) {
+ lines.add(this.formatGlobalFeedbackAnnotations(annotationsWithoutScore, false));
}
}
}
@@ -580,8 +661,11 @@ private List createGlobalFeedback(RatingGroup ratingGroup) {
List feedbacks = new ArrayList<>();
for (int i = 0; i < feedbackTexts.size(); i++) {
// Only the first feedback deducts points
- feedbacks.add(FeedbackDTO.newVisibleManualUnreferenced(
- i == 0 ? points.score() : 0.0, null, feedbackTexts.get(i)));
+ double score = points.score();
+ if (i != 0) {
+ score = GLOBAL_FEEDBACK_OTHER_PARTS_SCORE;
+ }
+ feedbacks.add(FeedbackDTO.newVisibleManualUnreferenced(score, null, feedbackTexts.get(i)));
}
return feedbacks;
}
diff --git a/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/location/ComparatorUtils.java b/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/location/ComparatorUtils.java
index 85ef859..c61026d 100644
--- a/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/location/ComparatorUtils.java
+++ b/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/location/ComparatorUtils.java
@@ -4,30 +4,28 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.Comparator;
-import java.util.function.Function;
-final class ComparatorUtils {
+public final class ComparatorUtils {
private ComparatorUtils() {}
/**
- * This is essentially {@link Comparator#comparing(Function)}, but for collections, which don't implement {@link Comparable} by default.
+ * Returns a comparator that compares collections based on their elements.
*
- * Make sure that the provided collection has some kind of order, like a {@link java.util.TreeSet} or a {@link java.util.List}.
- * The behavior is undefined if the collection is unordered.
+ * The comparator will first compare the elements element-wise, and if they are equal, the collection with fewer elements
+ * is considered smaller.
*
- * @param keyExtractor the function to extract the key from the object
- * @return a comparator that compares the objects based on the extracted keys
- * @param the type of the objects to compare
- * @param the type of the keys to compare
+ * @param comparator the comparator to compare the elements with
+ * @return a comparator that compares collections based on their elements
+ * @param the type of the elements in the collections
+ * @param the type of the collections
*/
- static > Comparator comparing(
- Function> keyExtractor) {
+ public static > Comparator compareByElement(Comparator comparator) {
return (left, right) -> {
- var leftList = new ArrayList<>(keyExtractor.apply(left));
- var rightList = new ArrayList<>(keyExtractor.apply(right));
+ var leftList = new ArrayList<>(left);
+ var rightList = new ArrayList<>(right);
for (int i = 0; i < Math.min(leftList.size(), rightList.size()); i++) {
- int comparison = leftList.get(i).compareTo(rightList.get(i));
+ int comparison = comparator.compare(leftList.get(i), rightList.get(i));
if (comparison != 0) {
return comparison;
}
@@ -36,4 +34,21 @@ static > Comparator comparing(
return Integer.compare(leftList.size(), rightList.size());
};
}
+
+ static , U extends Collection> Comparator compareByElement() {
+ return compareByElement(Comparator.naturalOrder());
+ }
+
+ /**
+ * Returns a comparator that compares collections based on their size.
+ *
+ * @param comparator the comparator to compare collections with the same size
+ * @return a comparator that compares collections based on their size
+ * @param the type of the elements in the collections
+ * @param the type of the collections
+ */
+ public static > Comparator shortestFirst(Comparator comparator) {
+ // NOTE: does no longer compile if the lambda is replaced with Collection::size
+ return Comparator.comparingInt((U collection) -> collection.size()).thenComparing(comparator);
+ }
}
diff --git a/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/location/LocationFormatter.java b/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/location/LocationFormatter.java
index 4698625..0eb5926 100644
--- a/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/location/LocationFormatter.java
+++ b/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/location/LocationFormatter.java
@@ -1,6 +1,7 @@
/* Licensed under EPL-2.0 2025. */
package edu.kit.kastel.sdq.artemis4j.grading.location;
+import java.util.Comparator;
import java.util.List;
import java.util.SequencedSet;
import java.util.Set;
@@ -145,7 +146,8 @@ public int hashCode() {
@Override
public int compareTo(LocationFormatter other) {
// Comparable is mostly implemented for convenience in intelligrade.
- return ComparatorUtils.comparing(LocationFormatter::segments).compare(this, other);
+ return Comparator.comparing(LocationFormatter::segments, ComparatorUtils.compareByElement())
+ .compare(this, other);
}
/**
diff --git a/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/location/PathFormatter.java b/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/location/PathFormatter.java
index 79bbaeb..0d6e0e3 100644
--- a/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/location/PathFormatter.java
+++ b/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/location/PathFormatter.java
@@ -14,7 +14,6 @@ class PathFormatter {
PathFormatter() {
this.shouldMergeLines = false;
this.showFilePath = true;
-
}
PathFormatter shouldMergeLines(boolean shouldMergeLines) {
diff --git a/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/location/PathSegment.java b/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/location/PathSegment.java
index 6862500..ec6542e 100644
--- a/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/location/PathSegment.java
+++ b/src/main/java/edu/kit/kastel/sdq/artemis4j/grading/location/PathSegment.java
@@ -80,8 +80,8 @@ public int compareTo(PathSegment other) {
// ^ because the file does not have elements, it should be considered smaller
// 3. if both are files, compare by locations
return Comparator.comparing(PathSegment::name)
- .thenComparing(ComparatorUtils.comparing(PathSegment::elements))
- .thenComparing(ComparatorUtils.comparing(PathSegment::locations))
+ .thenComparing(PathSegment::elements, ComparatorUtils.compareByElement())
+ .thenComparing(PathSegment::locations, ComparatorUtils.compareByElement())
.compare(this, other);
}
diff --git a/src/test/java/edu/kit/kastel/sdq/artemis4j/End2EndTest.java b/src/test/java/edu/kit/kastel/sdq/artemis4j/End2EndTest.java
index 761b714..7266b21 100644
--- a/src/test/java/edu/kit/kastel/sdq/artemis4j/End2EndTest.java
+++ b/src/test/java/edu/kit/kastel/sdq/artemis4j/End2EndTest.java
@@ -7,6 +7,8 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
import java.util.List;
import java.util.Optional;
@@ -361,4 +363,98 @@ void testHighlight() {
MistakeType.Highlight.NONE,
this.gradingConfig.getMistakeTypeById("magicLiteral").getHighlight());
}
+
+ @Test
+ void testGlobalFeedbackSorting() throws ArtemisClientException {
+ // First add some annotations
+ String defaultFeedbackText = "Some feedback text";
+
+ for (int i = 0; i < 5; i++) {
+ this.assessment.addPredefinedAnnotation(
+ this.gradingConfig.getMistakeTypeById("jdEmpty"),
+ new Location("src/edu/kit/informatik/BubbleSort.java", i, i),
+ defaultFeedbackText);
+ }
+
+ this.assessment.addPredefinedAnnotation(
+ this.gradingConfig.getMistakeTypeById("jdEmpty"),
+ new Location("src/edu/kit/informatik/InsertionSort.java", 5, 5),
+ defaultFeedbackText);
+
+ for (int i = 0; i < 3; i++) {
+ this.assessment.addPredefinedAnnotation(
+ this.gradingConfig.getMistakeTypeById("jdEmpty"),
+ new Location("src/edu/kit/informatik/RadixSort.java", i + 3, i + 5),
+ defaultFeedbackText);
+ }
+
+ // to test custom scoring, add some custom annotations:
+ for (int i = 0; i < 3; i++) {
+ this.assessment.addCustomAnnotation(
+ this.gradingConfig.getMistakeTypeById("custom"),
+ new Location("src/edu/kit/informatik/RadixSort.java", i, i),
+ defaultFeedbackText,
+ -0.5);
+ }
+
+ this.assessment.addCustomAnnotation(
+ this.gradingConfig.getMistakeTypeById("custom"),
+ new Location("src/edu/kit/informatik/InsertionSort.java", 5, 5),
+ defaultFeedbackText,
+ 0.0);
+
+ for (int i = 0; i < 2; i++) {
+ this.assessment.addCustomAnnotation(
+ this.gradingConfig.getMistakeTypeById("custom"),
+ new Location("src/edu/kit/informatik/BubbleSort.java", i, i),
+ defaultFeedbackText,
+ 0.0);
+ }
+
+ this.assessment.addCustomAnnotation(
+ this.gradingConfig.getMistakeTypeById("custom"),
+ new Location("src/edu/kit/informatik/BubbleSort.java", 3, 3),
+ defaultFeedbackText,
+ -0.5);
+
+ // this should be the first annotation
+ this.assessment.addPredefinedAnnotation(
+ this.gradingConfig.getMistakeTypeById("jdTrivial"),
+ new Location("src/edu/kit/informatik/BubbleSort.java", 0, 0),
+ defaultFeedbackText);
+
+ this.assessment.submit();
+ this.assessment = this.programmingSubmission.tryLock(this.gradingConfig).orElseThrow();
+
+ ResultDTO resultDTO = this.programmingSubmission.getRelevantResult().orElseThrow();
+ var feedbacks = ResultDTO.fetchDetailedFeedbacks(
+ this.connection.getClient(),
+ resultDTO.id(),
+ this.programmingSubmission.getParticipationId(),
+ resultDTO.feedbacks());
+
+ Collection globalFeedbackLines = new ArrayList<>();
+ for (FeedbackDTO feedbackDTO : feedbacks) {
+ if (feedbackDTO.type() != FeedbackType.MANUAL_UNREFERENCED || feedbackDTO.visibility() != null) {
+ continue;
+ }
+
+ globalFeedbackLines.addAll(Arrays.asList(feedbackDTO.detailText().split("\\n")));
+ }
+
+ assertEquals(
+ List.of(
+ "Funktionalität [-12P (Range: -20P -- ∞P)]",
+ " * JavaDoc Trivial [-5P]:",
+ " * src/edu/kit/informatik/BubbleSort.java at line 1",
+ " * Custom Penalty [-2P]:",
+ " * src/edu/kit/informatik/RadixSort.java at lines 1, 2, 3 (-1,5P)",
+ " * src/edu/kit/informatik/InsertionSort.java at line 6 (0P)",
+ " * src/edu/kit/informatik/BubbleSort.java at lines 1, 2, 4 (-0,5P)",
+ " * JavaDoc Leer [-5P]:",
+ " * src/edu/kit/informatik/BubbleSort.java at lines 1, 2, 3, 4, 5",
+ " * src/edu/kit/informatik/InsertionSort.java at line 6",
+ " * src/edu/kit/informatik/RadixSort.java at lines 4, 5, 6"),
+ globalFeedbackLines);
+ }
}