Adaptive learning: Send competencies to Athena for feedback
#11458
Conversation
github-actions
bot
added
server
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||
WalkthroughAdds course competency support: new CourseCompetencyDTO and CourseCompetencyApi method, and updates AthenaFeedbackSuggestionsService to accept an optional CourseCompetencyApi, fetch/map competencies per exercise, compute latestSubmission, and include both in the RequestDTO used by feedback suggestion flows. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant Service as AthenaFeedbackSuggestionsService
participant API as CourseCompetencyApi
participant Repo as CourseCompetencyRepository
participant Builder as RequestDTO
participant Engine as SuggestionEngine
Caller->>Service: get*FeedbackSuggestions(exerciseId, ...)
rect rgba(230,240,255,0.6)
note right of Service: compute latestSubmission (if available)
end
alt CourseCompetencyApi present
Service->>API: findAllByExerciseIdWithExerciseLinks(exerciseId)
API->>Repo: findAllByExerciseIdWithExerciseLinks(exerciseId)
Repo-->>API: List<CourseCompetency>
API-->>Service: List<CourseCompetency>
Service->>Service: Map to List<CourseCompetencyDTO>
else CourseCompetencyApi absent
Service->>Service: competencies = null
end
Service->>Builder: Build RequestDTO(..., latestSubmission, competencies)
Builder-->>Service: RequestDTO
Service->>Engine: send RequestDTO
Engine-->>Service: suggestions
Service-->>Caller: suggestions
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java (1)
32-248: Consider factoring out competency lookups.The new competency mapping logic repeats in all three request builders. A small helper such as
private List<CourseCompetencyDTO> resolveCompetencies(long exerciseId)would keep the methods focused on request assembly and reduce duplication. Not urgent, but it would simplify future tweaks.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java(7 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/api/CourseCompetencyApi.java(2 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/dto/CourseCompetencyDTO.java(1 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/repository/CourseCompetencyRepository.java(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
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
Files:
src/main/java/de/tum/cit/aet/artemis/atlas/api/CourseCompetencyApi.javasrc/main/java/de/tum/cit/aet/artemis/atlas/dto/CourseCompetencyDTO.javasrc/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.javasrc/main/java/de/tum/cit/aet/artemis/atlas/repository/CourseCompetencyRepository.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: Mend Security Check
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: Analyse
🔇 Additional comments (3)
src/main/java/de/tum/cit/aet/artemis/atlas/repository/CourseCompetencyRepository.java (1)
318-320: Good reuse of existing fetch logic.Leaning on
findAllByExerciseIdWithExerciseLinkskeeps behavior consistent while exposing the simpler API.src/main/java/de/tum/cit/aet/artemis/atlas/api/CourseCompetencyApi.java (1)
3-30: Controller delegation looks spot on.The API surface simply forwards to the repository, which is exactly what we need here—no extra logic leaked.
src/main/java/de/tum/cit/aet/artemis/atlas/dto/CourseCompetencyDTO.java (1)
1-14: DTO implementation looks clean.Using a record with
JsonInclude.NON_EMPTYkeeps the payload lean, and theoffactory is a tidy mapper.
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
src/main/java/de/tum/cit/aet/artemis/atlas/repository/CourseCompetencyRepository.java
Outdated
Show resolved
Hide resolved
github-project-automation
bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/de/tum/cit/aet/artemis/atlas/api/CourseCompetencyApi.java (1)
28-30: Consider adding JavaDoc for the public API method.The method implementation is clean and follows the existing delegation pattern. However, the PR checklist indicates "JavaDoc added," yet this public method lacks documentation. Adding JavaDoc would clarify the method's purpose, parameter constraints (e.g., valid exercise IDs), and return value behavior (empty list vs. populated list).
Apply this diff to add JavaDoc:
+ /** + * Finds all course competencies linked to the specified exercise. + * + * @param exerciseId the ID of the exercise + * @return a list of course competencies linked to the exercise, or an empty list if none exist + */ public List<CourseCompetency> findAllByExerciseId(long exerciseId) { return courseCompetencyRepository.findAllByExerciseIdWithExerciseLinks(exerciseId); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/de/tum/cit/aet/artemis/atlas/api/CourseCompetencyApi.java(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
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
Files:
src/main/java/de/tum/cit/aet/artemis/atlas/api/CourseCompetencyApi.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: client-style
- GitHub Check: bean-instantiation-check
- GitHub Check: client-tests
- GitHub Check: server-tests
- GitHub Check: server-style
- GitHub Check: Analyse
🔇 Additional comments (1)
src/main/java/de/tum/cit/aet/artemis/atlas/api/CourseCompetencyApi.java (1)
3-3: LGTM!The import is necessary for the new method's return type and follows standard Java conventions.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java (1)
191-192: Consider adding error handling for competency fetching.The
extractLearnerProfile()method includes try-catch error handling (lines 163-169), but the competency fetching logic does not. For consistency and robustness, consider wrapping the competency API call in similar error handling.If you extract the helper method as suggested above, add error handling:
private List<CourseCompetencyDTO> getCompetenciesForExercise(long exerciseId) { try { return courseCompetencyApi .map(api -> api.findAllByExerciseIdWithExerciseLinks(exerciseId).stream() .map(CourseCompetencyDTO::of) .toList()) .orElse(null); } catch (Exception e) { log.error("Error retrieving competencies for exercise {}: {}", exerciseId, e.getMessage()); return null; } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java(7 hunks)src/main/java/de/tum/cit/aet/artemis/atlas/api/CourseCompetencyApi.java(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
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
Files:
src/main/java/de/tum/cit/aet/artemis/atlas/api/CourseCompetencyApi.javasrc/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
- GitHub Check: client-style
- GitHub Check: client-tests
- GitHub Check: server-style
- GitHub Check: server-tests
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: Analyse
- GitHub Check: bean-instantiation-check
🔇 Additional comments (2)
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java (2)
213-218: LGTM: Consistent implementation across feedback methods.The addition of
latestSubmissionandcompetenciesto the programming feedback flow follows the same pattern as the text feedback flow, ensuring consistency across exercise types.
241-247: LGTM: Consistent implementation across feedback methods.The addition of
latestSubmissionandcompetenciesto the modeling feedback flow follows the same pattern as the other feedback flows, ensuring consistency across exercise types.
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/athena/service/AthenaFeedbackSuggestionsService.java
Show resolved
Hide resolved
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
helios-aet
bot
temporarily deployed
to
artemis-test1.artemis.cit.tum.de
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
Well, I need to fetch the competencies that are linked to an exercise, and as far as I understand, I can do that through exercise links. @MaximilianAnzinger |
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
src/main/java/de/tum/cit/aet/artemis/atlas/dto/CourseCompetencyDTO.java
Outdated
Show resolved
Hide resolved
ahmetsenturk
dismissed stale reviews from florian-glombik, tobias-lippert, and coderabbitai[bot]
via
57ec86c
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/de/tum/cit/aet/artemis/atlas/dto/CourseCompetencyDTO.java (1)
14-16: Align null-safety inof(CourseCompetency)with other DTOs
Either annotate thecompetencyparameter with@NotNulland add a null check (as inCompetencyJolDTO.of) or consistently omit parameter validation across allof(...)methods.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/de/tum/cit/aet/artemis/atlas/dto/CourseCompetencyDTO.java(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/main/java/**/*.java
⚙️ CodeRabbit configuration file
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
Files:
src/main/java/de/tum/cit/aet/artemis/atlas/dto/CourseCompetencyDTO.java
🧠 Learnings (1)
📚 Learning: 2024-07-02T21:16:24.494Z
Learnt from: JohannesStoehr
PR: ls1intum/Artemis#8947
File: src/main/java/de/tum/in/www1/artemis/service/ExerciseService.java:760-762
Timestamp: 2024-07-02T21:16:24.494Z
Learning: Even when the `NotNull` annotation is used, adding a null check in methods is recommended to catch potential issues early and provide clear error messages.
Applied to files:
src/main/java/de/tum/cit/aet/artemis/atlas/dto/CourseCompetencyDTO.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: server-style
- GitHub Check: server-tests
- GitHub Check: client-style
- GitHub Check: client-tests
- GitHub Check: Build and Push Docker Image / Build linux/amd64 Docker Image for ls1intum/artemis
- GitHub Check: Build and Push Docker Image / Build linux/arm64 Docker Image for ls1intum/artemis
- GitHub Check: Build .war artifact
- GitHub Check: Mend Security Check
- GitHub Check: bean-instantiation-check
- GitHub Check: Analyse
🔇 Additional comments (1)
src/main/java/de/tum/cit/aet/artemis/atlas/dto/CourseCompetencyDTO.java (1)
11-12: LGTM!The record definition follows best practices:
- Uses Java record as per coding guidelines
- All non-primitive fields are properly annotated with
@NotNullor@Nullableas per past review guidance- Uses primitive
longforid(prefer_primitives guideline)- Minimal data approach (single responsibility)
@JsonInclude(NON_EMPTY)appropriately excludes empty fields from JSON
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
4 participants
Checklist
General
Server
Motivation and Context
When Athena generates feedback, it makes use of competencies that are linked to exercises - so these competencies need to be passed to Athena.
Description
This PR extracts the competencies that are linked to an exercise and send to Athena when a feedback is requested.
Steps for Testing
Prerequisites:
Testserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Code Review
Manual Tests
Test Coverage
Summary by CodeRabbit
New Features
Bug Fixes