Adaptive learning: Use competency link weight for learning path recommendations and mastery calculation
#9565
Conversation
…etency-link-weight
…-weight # Conflicts: # src/main/java/de/tum/cit/aet/artemis/atlas/service/competency/CompetencyService.java # src/main/java/de/tum/cit/aet/artemis/atlas/service/competency/CourseCompetencyService.java # src/main/java/de/tum/cit/aet/artemis/atlas/service/competency/PrerequisiteService.java # src/test/javascript/spec/component/competencies/competency-management/competency-management.component.spec.ts
github-actions
bot
added
server
JohannesStoehr
changed the title
Adaptive Learning: Use competency link weight for learning path recommendations and mastery calculationAdaptive learning: Use competency link weight for learning path recommendations and mastery calculation
…feature/adaptive-learning/competency-link-weight-usage
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
…-weight # Conflicts: # src/main/webapp/app/exercises/programming/manage/update/update-components/programming-exercise-problem.component.html
…feature/adaptive-learning/competency-link-weight-usage
bassner
dismissed stale reviews from MaximilianAnzinger, dmytropolityka, HawKhiem, az108, florian-glombik, sachmii, and coderabbitai[bot]
The base branch was changed.
…-weight-usage # Conflicts: # src/main/java/de/tum/cit/aet/artemis/atlas/repository/CourseCompetencyRepository.java # src/main/java/de/tum/cit/aet/artemis/atlas/service/competency/CompetencyProgressService.java # src/main/java/de/tum/cit/aet/artemis/atlas/service/learningpath/LearningPathRecommendationService.java
WalkthroughThe pull request introduces several modifications across multiple files, primarily focusing on competency progress and mastery calculations. Key changes include the addition of new enum constants in the Changes
Assessment against linked issues
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (9)
src/main/java/de/tum/cit/aet/artemis/atlas/dto/metrics/CompetencyExerciseMasteryCalculationDTO.java (1)
10-11: Add Javadoc to document the new fields.The record structure looks good and follows the DTO guidelines. Consider adding Javadoc to document the purpose and constraints of the new fields
exerciseIdandcompetencyLinkWeight.+/** + * DTO for exercise mastery calculation containing exercise metadata and competency-specific information. + * + * @param exerciseId The unique identifier of the exercise + * @param maxPoints Maximum points achievable in the exercise + * @param difficulty The difficulty level of the exercise + * @param isProgrammingExercise Whether this is a programming exercise + * @param competencyLinkWeight The weight of the link between exercise and competency (must be positive) + * @param lastScore The last achieved score (null if not attempted) + * @param lastPoints The last achieved points (null if not attempted) + * @param lastModifiedDate The date of the last modification + * @param submissionCount Number of submissions made + */src/main/java/de/tum/cit/aet/artemis/atlas/domain/CompetencyProgressConfidenceReason.java (1)
11-11: Consider improving enum readability and documentation.While the new constants
MORE_LOW_WEIGHTED_EXERCISESandMORE_HIGH_WEIGHTED_EXERCISESare well-named and logically grouped, consider:
- Formatting the enum constants for better readability
- Adding Javadoc for each constant to document their specific use cases
Apply this formatting:
- NO_REASON, RECENT_SCORES_LOWER, RECENT_SCORES_HIGHER, MORE_EASY_POINTS, MORE_HARD_POINTS, QUICKLY_SOLVED_EXERCISES, MORE_LOW_WEIGHTED_EXERCISES, MORE_HIGH_WEIGHTED_EXERCISES + NO_REASON, + RECENT_SCORES_LOWER, + RECENT_SCORES_HIGHER, + MORE_EASY_POINTS, + MORE_HARD_POINTS, + QUICKLY_SOLVED_EXERCISES, + MORE_LOW_WEIGHTED_EXERCISES, + MORE_HIGH_WEIGHTED_EXERCISES; + + /** No specific reason for confidence adjustment */ + NO_REASON, + + /** Recent exercise scores are lower than expected */ + RECENT_SCORES_LOWER, + + /** Recent exercise scores are higher than expected */ + RECENT_SCORES_HIGHER, + + /** More points earned from easier exercises */ + MORE_EASY_POINTS, + + /** More points earned from harder exercises */ + MORE_HARD_POINTS, + + /** Exercises solved faster than expected */ + QUICKLY_SOLVED_EXERCISES, + + /** More points earned from exercises with lower competency link weights */ + MORE_LOW_WEIGHTED_EXERCISES, + + /** More points earned from exercises with higher competency link weights */ + MORE_HIGH_WEIGHTED_EXERCISESsrc/main/webapp/app/entities/competency.model.ts (1)
Line range hint
52-52: Add JSDoc comment for MEDIUM_COMPETENCY_LINK_WEIGHT constant.Since this constant is crucial for competency link weight calculations, consider adding documentation to explain its significance and usage.
+/** + * The reference point for medium competency link weights (0.5). + * Used to determine if an exercise has high (> 0.5) or low (< 0.5) weight + * in competency mastery calculations. + */ export const MEDIUM_COMPETENCY_LINK_WEIGHT = 0.5;src/main/java/de/tum/cit/aet/artemis/atlas/repository/CourseCompetencyRepository.java (2)
133-156: LGTM! Well-structured query with proper documentation.The implementation follows the same pattern as the exercise info query, properly incorporating competency link weights and completion status.
There appears to be a redundant
uat the end of the GROUP BY clause. Consider removing it:- GROUP BY lu.id, u, lul.weight, u + GROUP BY lu.id, u, lul.weight
Line range hint
110-156: Architecture looks solid for weight-based mastery calculations.The repository changes effectively support the PR objectives by:
- Including competency link weights in both exercise and lecture unit queries
- Maintaining consistent query patterns between both learning object types
- Providing the necessary data for enhanced learning path recommendations
This implementation allows for flexible weight-based calculations while maintaining good separation of concerns.
src/main/java/de/tum/cit/aet/artemis/atlas/service/learningpath/LearningPathRecommendationService.java (1)
639-642: Consider using Map.of() for a more concise initialization.The implementation is correct, but you could make it more concise using Map.of():
-Map<DifficultyLevel, List<Exercise>> difficultyLevelMap = new HashMap<>(); -for (var difficulty : DifficultyLevel.values()) { - difficultyLevelMap.put(difficulty, new ArrayList<>()); -} +Map<DifficultyLevel, List<Exercise>> difficultyLevelMap = Stream.of(DifficultyLevel.values()) + .collect(Collectors.toMap( + difficulty -> difficulty, + difficulty -> new ArrayList<>() + ));🧰 Tools
🪛 ast-grep
[warning] 641-641: Detected a cookie where the
Secureflag is either missing or disabled. TheSecurecookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set theSecureflag totrueso the cookie will only be sent over HTTPS.
Context: (difficulty, new ArrayList<>())
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 641-641: Detected a cookie where the
HttpOnlyflag is either missing or disabled. TheHttpOnlycookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set theHttpOnlyflag to true` in all other cases.
Context: (difficulty, new ArrayList<>())
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfigurationsrc/main/java/de/tum/cit/aet/artemis/atlas/service/competency/CompetencyProgressService.java (3)
Line range hint
258-269: Fix potential division by zero in lecture progress calculationIn the
calculateProgressmethod, there is a potential division by zero whenlectureUnitInfos.size()is zero at line 269. This can cause a runtime exception.Apply this diff to prevent division by zero:
double numberOfLearningObjects = lectureUnitInfos.size() + exerciseInfos.size(); if (numberOfLearningObjects == 0) { // If nothing is linked to the competency, the competency is considered completed competencyProgress.setProgress(100.0); + return; } +long numberOfCompletedLectureUnits = 0; +double lectureProgress = 0.0; +if (lectureUnitInfos.size() > 0) { + numberOfCompletedLectureUnits = lectureUnitInfos.stream().filter(CompetencyLectureUnitMasteryCalculationDTO::completed).count(); + lectureProgress = 100.0 * numberOfCompletedLectureUnits / lectureUnitInfos.size(); +}
Line range hint
256-272: Refactor duplicate logic in progress calculationsThe methods
calculateProgress(lines 256-272) andcalculateCompetencyLinkWeightConfidenceHeuristic(lines 422-447) contain similar logic for calculating progress, differing mainly by whether they consider competency link weights.Consider refactoring to extract the common calculation steps into a shared helper method that can handle both weighted and unweighted cases. This will reduce code duplication and enhance maintainability.
Also applies to: 422-447
474-483: Clarify tie-breaker logic in confidence reason determinationIn the methods
setConfidenceReasonLow(lines 474-483) andsetConfidenceReasonHigh(lines 488-501), when determining the minimum or maximum confidence heuristic, ties between heuristics may occur.To ensure consistent behavior, explicitly handle tie cases or document the intended tie-breaker logic. This will make the code more predictable and easier to understand.
Also applies to: 488-501
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (10)
src/main/java/de/tum/cit/aet/artemis/atlas/domain/CompetencyProgressConfidenceReason.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/dto/metrics/CompetencyExerciseMasteryCalculationDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/dto/metrics/CompetencyLectureUnitMasteryCalculationDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/repository/CourseCompetencyRepository.java(3 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/service/competency/CompetencyProgressService.java(9 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/service/learningpath/LearningPathRecommendationService.java(6 hunks)src/main/java/de/tum/cit/aet/artemis/lecture/repository/LectureUnitCompletionRepository.java(0 hunks)src/main/webapp/app/entities/competency.model.ts(1 hunks)src/main/webapp/i18n/de/competency.json(1 hunks)src/main/webapp/i18n/en/competency.json(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/de/tum/cit/aet/artemis/lecture/repository/LectureUnitCompletionRepository.java
✅ Files skipped from review due to trivial changes (1)
- src/main/java/de/tum/cit/aet/artemis/atlas/dto/metrics/CompetencyLectureUnitMasteryCalculationDTO.java
🧰 Additional context used
📓 Path-based instructions (7)
src/main/java/de/tum/cit/aet/artemis/atlas/domain/CompetencyProgressConfidenceReason.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
src/main/java/de/tum/cit/aet/artemis/atlas/dto/metrics/CompetencyExerciseMasteryCalculationDTO.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
src/main/java/de/tum/cit/aet/artemis/atlas/repository/CourseCompetencyRepository.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
src/main/java/de/tum/cit/aet/artemis/atlas/service/competency/CompetencyProgressService.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
src/main/java/de/tum/cit/aet/artemis/atlas/service/learningpath/LearningPathRecommendationService.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
src/main/webapp/app/entities/competency.model.ts (1)
src/main/webapp/i18n/de/competency.json (1)
Pattern src/main/webapp/i18n/de/**/*.json: German language translations should be informal (dutzen) and should never be formal (sietzen). So the user should always be addressed with "du/dein" and never with "sie/ihr".
📓 Learnings (1)
src/main/java/de/tum/cit/aet/artemis/atlas/service/competency/CompetencyProgressService.java (2)
Learnt from: julian-christl
PR: ls1intum/Artemis#8052
File: src/test/java/de/tum/in/www1/artemis/lecture/CompetencyIntegrationTest.java:310-310
Timestamp: 2024-02-23T00:03:06.365Z
Learning: Modifications to parameters in `competencyProgressUtilService.createCompetencyProgress` for debugging purposes are considered irrelevant to the test outcomes but helpful for clarity during debugging.
Learnt from: julian-christl
PR: ls1intum/Artemis#8052
File: src/test/java/de/tum/in/www1/artemis/lecture/CompetencyIntegrationTest.java:310-310
Timestamp: 2024-10-08T15:35:52.595Z
Learning: Modifications to parameters in `competencyProgressUtilService.createCompetencyProgress` for debugging purposes are considered irrelevant to the test outcomes but helpful for clarity during debugging.
🪛 ast-grep
src/main/java/de/tum/cit/aet/artemis/atlas/service/learningpath/LearningPathRecommendationService.java
[warning] 641-641: Detected a cookie where the Secure flag is either missing or disabled. The Secure cookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set the Secure flag to true so the cookie will only be sent over HTTPS.
Context: (difficulty, new ArrayList<>())
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 641-641: Detected a cookie where the HttpOnly flag is either missing or disabled. The HttpOnly cookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set the HttpOnly flag to true` in all other cases.
Context: (difficulty, new ArrayList<>())
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
🔇 Additional comments (7)
src/main/java/de/tum/cit/aet/artemis/atlas/domain/CompetencyProgressConfidenceReason.java (1)
8-8: LGTM! Documentation reference update aligns with architectural changes.
The update to reference CompetencyProgressService#setConfidenceReason correctly reflects the movement of confidence reason logic to the service layer, adhering to the single responsibility principle.
src/main/webapp/app/entities/competency.model.ts (1)
178-179: LGTM! Verify enum value usage in translations.
The new enum values follow the correct naming convention and align with the PR's objective of incorporating competency link weights into mastery calculations.
Let's verify that these new enum values have corresponding translations:
✅ Verification successful
✓ Translation entries are properly configured for both enum values
The new enum values MORE_LOW_WEIGHTED_EXERCISES and MORE_HIGH_WEIGHTED_EXERCISES have corresponding translations in both English and German language files with clear, descriptive messages that accurately explain the weight-based mastery calculation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for translation entries for the new confidence reasons
# Expected: Both enum values should have corresponding entries in i18n files
echo "Checking English translations..."
rg "MORE_LOW_WEIGHTED_EXERCISES|MORE_HIGH_WEIGHTED_EXERCISES" "src/main/webapp/i18n/en/competency.json"
echo "Checking German translations..."
rg "MORE_LOW_WEIGHTED_EXERCISES|MORE_HIGH_WEIGHTED_EXERCISES" "src/main/webapp/i18n/de/competency.json"
Length of output: 1306
src/main/java/de/tum/cit/aet/artemis/atlas/repository/CourseCompetencyRepository.java (1)
Line range hint 110-131: LGTM! Query changes look correct.
The addition of e.id to both SELECT and GROUP BY clauses ensures proper result grouping, and the method rename better reflects its functionality.
Let's verify that all callers have been updated to use the new method name:
✅ Verification successful
LGTM! Method is correctly used throughout the codebase.
The search results show that:
- The method
findAllExerciseInfoByCompetencyIdAndUseris only used inCompetencyProgressService.javawhich correctly uses the new method name with both parameters. - All usages of
CompetencyExerciseMasteryCalculationDTOare consistent with the method's return type and parameter structure. - The method is integrated properly into the competency progress calculation logic.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining calls to the old method name
rg "findAllExerciseInfoByCompetencyId\(" --type java
Length of output: 54
Script:
#!/bin/bash
# Let's try a broader search to find any references to this method, including potential callers
rg -i "findAllExerciseInfoByCompetency" --type java -A 2 -B 2
# Also search for the DTO to find potential callers
rg "CompetencyExerciseMasteryCalculationDTO" --type java -A 2 -B 2
Length of output: 17533
src/main/webapp/i18n/en/competency.json (1)
28-30: LGTM! Clear and well-structured confidence reason messages.
The new messages effectively communicate how exercise weights influence mastery calculations, maintaining consistency with existing message patterns while clearly explaining the relationship between progress and mastery scores.
src/main/webapp/i18n/de/competency.json (1)
28-30: LGTM! Translations follow guidelines and maintain consistency.
The new translations correctly:
- Use informal tone ("du" form) as required
- Maintain consistent style with existing confidence reasons
- Clearly explain the relationship between exercise weights and mastery scores
src/main/java/de/tum/cit/aet/artemis/atlas/service/learningpath/LearningPathRecommendationService.java (2)
493-496: LGTM: Exercise ordering implementation effectively addresses inconsistency issue.
The double sorting approach elegantly solves two requirements:
- Consistent ordering (by title) prevents the exercise sequence from changing unexpectedly
- Weight-based prioritization (descending) ensures exercises with higher competency link weights are recommended first
639-642: Verify callers handle List return type.
Let's verify that changing the return type from Set to List doesn't break any callers.
✅ Verification successful
Based on the search results, I can generate the final response as we have sufficient information:
No issues found with List return type usage
The method generateDifficultyLevelMap is only used once within the same class, and the caller already expects a Map<DifficultyLevel, List<Exercise>> return type. No Set type expectations were found in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find callers of generateDifficultyLevelMap and verify they handle List return type
# Test: Search for method calls and their context
rg -A 5 "generateDifficultyLevelMap"
# Test: Look for potential Set type expectations in variable declarations
ast-grep --pattern 'Set<$_> $_ = generateDifficultyLevelMap'
Length of output: 2255
🧰 Tools
🪛 ast-grep
[warning] 641-641: Detected a cookie where the Secure flag is either missing or disabled. The Secure cookie flag instructs the browser to forbid sending the cookie over an insecure HTTP request. Set the Secure flag to true so the cookie will only be sent over HTTPS.
Context: (difficulty, new ArrayList<>())
Note: [CWE-614]: Sensitive Cookie in HTTPS Session Without 'Secure' Attribute [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
[warning] 641-641: Detected a cookie where the HttpOnly flag is either missing or disabled. The HttpOnly cookie flag instructs the browser to forbid client-side JavaScript to read the cookie. If JavaScript interaction is required, you can ignore this finding. However, set the HttpOnly flag to true` in all other cases.
Context: (difficulty, new ArrayList<>())
Note: [CWE-1004]: Sensitive Cookie Without 'HttpOnly' Flag [OWASP A05:2021]: Security Misconfiguration [REFERENCES]
- https://owasp.org/Top10/A05_2021-Security_Misconfiguration
...n/java/de/tum/cit/aet/artemis/atlas/dto/metrics/CompetencyExerciseMasteryCalculationDTO.java
Show resolved
Hide resolved
|
The base branch was changed so all reviews are lost. This PR had enough approvals. |
bassner
deleted the
feature/adaptive-learning/competency-link-weight-usage
branch
github-actions
bot
added
deployment-error
🚨 Stacked on #9517 only merge after the other PR 🚨
Checklist
General
Server
Client
Motivation and Context
#9446
continuation of #9517
Description
#9517 changed the database and object structure to support weights between competencies and learning objects.
This PR now uses the link weight for learning path recommendations and the mastery calculation:
Steps for Testing
Prerequisites:
Hard to test manually, since other factors also affect the exercise recommendation/mastery calculation...
You can test if the recommendation and mastery value are still somewhat reasonable:
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
Performance Tests
Test Coverage
Server
Summary by CodeRabbit
Release Notes
New Features
MORE_LOW_WEIGHTED_EXERCISESandMORE_HIGH_WEIGHTED_EXERCISES.CompetencyLectureUnitMasteryCalculationDTOto enhance mastery calculation for lecture units.Improvements
Localization
Bug Fixes
LectureUnitCompletionRepository.