-
Notifications
You must be signed in to change notification settings - Fork 301
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
Text exercises
: Set the isGraded flag to false for student athena requests
#10155
base: develop
Are you sure you want to change the base?
Text exercises
: Set the isGraded flag to false for student athena requests
#10155
Conversation
Text-exercises
: Set the is_graded flag to false for student athena requestsText exercises
: Set the is_graded flag to false for student athena requests
Text exercises
: Set the is_graded flag to false for student athena requestsText exercises
: Set the isGraded flag to false for student athena requests
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 pmd (7.8.0)src/main/java/de/tum/cit/aet/artemis/text/service/TextExerciseFeedbackService.javaThe following rules are missing or misspelled in your ruleset file category/vm/bestpractices.xml: BooleanInstantiation, DontImportJavaLang, DuplicateImports, EmptyFinallyBlock, EmptyIfStmt, EmptyInitializer, EmptyStatementBlock, EmptyStatementNotInLoop, EmptySwitchStatements, EmptySynchronizedBlock, EmptyTryBlock, EmptyWhileStmt, ExcessiveClassLength, ExcessiveMethodLength, ImportFromSamePackage, MissingBreakInSwitch, SimplifyBooleanAssertion. Please check your ruleset configuration. WalkthroughThe pull request introduces a minor modification in the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/main/java/de/tum/cit/aet/artemis/text/service/TextExerciseFeedbackService.java (2)
Line range hint
73-196
: Consider breaking down the method for better maintainability.The
generateAutomaticNonGradedFeedback
method is quite long and handles multiple responsibilities. Consider extracting the feedback processing logic (lines 124-158) into a separate method for better maintainability and testability.Here's a suggested refactoring:
public void generateAutomaticNonGradedFeedback(StudentParticipation participation, TextExercise textExercise) { log.debug("Using athena to generate (text exercise) feedback request: {}", textExercise.getId()); // ... existing validation code ... Result automaticResult = new Result(); // ... result initialization ... try { this.resultWebsocketService.broadcastNewResult(participation, automaticResult); log.debug("Submission id: {}", textSubmission.getId()); var athenaResponse = this.athenaFeedbackSuggestionsService.orElseThrow() .getTextFeedbackSuggestions(textExercise, textSubmission, false); + var feedbackResult = processFeedbackSuggestions(athenaResponse, textSubmission); + updateResultWithFeedback(automaticResult, feedbackResult, textExercise); - Set<TextBlock> textBlocks = new HashSet<>(); - List<Feedback> feedbacks = new ArrayList<>(); - // ... feedback processing code ... automaticResult = this.resultRepository.save(automaticResult); // ... rest of the method ... } catch (Exception e) { // ... error handling ... } } + private record FeedbackResult(Set<TextBlock> textBlocks, List<Feedback> feedbacks) {} + + private FeedbackResult processFeedbackSuggestions(List<AthenaFeedbackItem> athenaResponse, TextSubmission textSubmission) { + Set<TextBlock> textBlocks = new HashSet<>(); + List<Feedback> feedbacks = new ArrayList<>(); + + athenaResponse.stream() + .filter(item -> item.description() != null) + .forEach(item -> { + var textBlock = new TextBlock(); + var feedback = new Feedback(); + // ... existing feedback processing logic ... + }); + + return new FeedbackResult(textBlocks, feedbacks); + } + + private void updateResultWithFeedback(Result result, FeedbackResult feedbackResult, TextExercise exercise) { + double totalFeedbacksScore = feedbackResult.feedbacks().stream() + .mapToDouble(Feedback::getCredits) + .sum(); + totalFeedbacksScore = totalFeedbacksScore / exercise.getMaxPoints() * 100; + result.setCompletionDate(ZonedDateTime.now()); + result.setScore(Math.clamp(totalFeedbacksScore, 0, 100)); + result.setSuccessful(true); + }
Line range hint
165-196
: Consider more specific exception handling.The catch block currently handles all exceptions the same way. Consider catching specific exceptions (e.g.,
AthenaServiceException
) to provide more targeted error handling and logging.Example:
- catch (Exception e) { + catch (AthenaServiceException e) { log.error("Could not generate feedback from Athena", e); // ... error handling ... + } catch (IllegalArgumentException e) { + log.error("Invalid input for feedback generation", e); + // ... error handling ... + } catch (Exception e) { + log.error("Unexpected error during feedback generation", e); // ... error handling ... }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/de/tum/cit/aet/artemis/text/service/TextExerciseFeedbackService.java
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/de/tum/cit/aet/artemis/text/service/TextExerciseFeedbackService.java (1)
Pattern src/main/java/**/*.java
: naming:CamelCase; principles:{single_responsibility,small_methods,no_duplication}; db:{perf_queries,datetime_not_timestamp}; rest:{stateless,singleton,delegate_logic,http_only,minimal_dtos}; dtos:{java_records,no_entities,min_data,single_resp}; di:constructor_injection; kiss:simple_code; file_handling:os_indep_paths; practices:{least_access,avoid_transactions,code_reuse,static_member_ref,prefer_primitives}; sql:{param_annotation,uppercase,avoid_subqueries};java:avoid_star_imports
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (1)
src/main/java/de/tum/cit/aet/artemis/text/service/TextExerciseFeedbackService.java (1)
124-124
: LGTM! The change aligns with the PR objectives.The modification correctly sets
isGraded
tofalse
when retrieving text feedback suggestions from Athena for student requests. This change maintains proper separation between graded (tutor) and non-graded (student) feedback scenarios.Let's verify that this is the only instance where we need to make this change:
✅ Verification successful
Verification complete - change is correctly isolated
The
isGraded=false
change inTextExerciseFeedbackService
is the only instance that needed modification because:
- It's the only endpoint handling student (non-graded) feedback requests
- Other occurrences are either tutor-protected endpoints or test cases simulating tutor scenarios
- Frontend calls are properly routed through their respective endpoints
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other potential calls to getTextFeedbackSuggestions that might need similar changes ast-grep --pattern 'getTextFeedbackSuggestions($_, $_, true)'Length of output: 187
Script:
#!/bin/bash # Search for all occurrences of getTextFeedbackSuggestions with context rg "getTextFeedbackSuggestions" -B 2 -A 2Length of output: 11812
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code
Checklist
General
Motivation and Context
The isGraded flag dictates whether the feedback was requested by a student or a tutor. This change is purely for Athena. This change has no effect in Artemis.
Description
This PR changes a single boolean flag from true to false in the TextExerciseFeedbackService.
Testserver States
Note
These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.
Review Progress
Performance Review
Code Review
Manual Tests
Exam Mode Test
Performance Tests
Test Coverage
Screenshots
Summary by CodeRabbit