-
Notifications
You must be signed in to change notification settings - Fork 387
Iris: Course Memory ingestion
#13002
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
base: develop
Are you sure you want to change the base?
Changes from all commits
49799cf
4e93924
5a3e9e6
1ca56f0
fb2d212
f67c260
9d73bd5
7c1d41b
6e8866c
4821ca3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,6 +48,7 @@ | |
| import de.tum.cit.aet.artemis.globalsearch.dto.searchableentity.PostSearchableEntityDTO; | ||
| import de.tum.cit.aet.artemis.globalsearch.service.SearchableEntityWeaviateService; | ||
| import de.tum.cit.aet.artemis.iris.api.AutonomousTutorApi; | ||
| import de.tum.cit.aet.artemis.iris.api.CourseMemoryIngestionApi; | ||
| import de.tum.cit.aet.artemis.notification.domain.course_notifications.NewAnswerNotification; | ||
| import de.tum.cit.aet.artemis.notification.domain.course_notifications.NewMentionNotification; | ||
| import de.tum.cit.aet.artemis.notification.service.CourseNotificationService; | ||
|
|
@@ -78,6 +79,8 @@ public class AnswerMessageService extends PostingService { | |
|
|
||
| private final Optional<AutonomousTutorApi> autonomousTutorApi; | ||
|
|
||
| private final Optional<CourseMemoryIngestionApi> courseMemoryIngestionApi; | ||
|
|
||
| private final TransactionTemplate transactionTemplate; | ||
|
|
||
| private final Optional<SearchableEntityWeaviateService> searchableEntityWeaviateService; | ||
|
|
@@ -88,7 +91,7 @@ public AnswerMessageService(SingleUserNotificationService singleUserNotification | |
| ConversationService conversationService, ExerciseRepository exerciseRepository, SavedPostRepository savedPostRepository, | ||
| WebsocketMessagingService websocketMessagingService, ConversationParticipantRepository conversationParticipantRepository, | ||
| ChannelAuthorizationService channelAuthorizationService, PostRepository postRepository, CourseNotificationService courseNotificationService, | ||
| Optional<AutonomousTutorApi> autonomousTutorApi, PlatformTransactionManager transactionManager, | ||
| Optional<AutonomousTutorApi> autonomousTutorApi, Optional<CourseMemoryIngestionApi> courseMemoryIngestionApi, PlatformTransactionManager transactionManager, | ||
| Optional<SearchableEntityWeaviateService> searchableEntityWeaviateService) { | ||
| super(courseRepository, userRepository, exerciseRepository, authorizationCheckService, websocketMessagingService, conversationParticipantRepository, savedPostRepository); | ||
| this.answerPostRepository = answerPostRepository; | ||
|
|
@@ -98,6 +101,7 @@ public AnswerMessageService(SingleUserNotificationService singleUserNotification | |
| this.singleUserNotificationService = singleUserNotificationService; | ||
| this.postRepository = postRepository; | ||
| this.courseNotificationService = courseNotificationService; | ||
| this.courseMemoryIngestionApi = courseMemoryIngestionApi; | ||
| this.autonomousTutorApi = autonomousTutorApi; | ||
| this.transactionTemplate = new TransactionTemplate(transactionManager); | ||
| this.searchableEntityWeaviateService = searchableEntityWeaviateService; | ||
|
|
@@ -229,14 +233,16 @@ public AnswerPost updateAnswerMessage(Long courseId, Long answerMessageId, Updat | |
| // only the content of the message can be updated | ||
| existingAnswerMessage.setContent(answerMessage.content()); | ||
|
|
||
| // determine if the update operation is to mark the answer message as resolving the original post | ||
| // determine if the update operation changes whether the answer message resolves the original post | ||
| boolean resolutionChanged = false; | ||
| if (existingAnswerMessage.doesResolvePost() != answerMessage.resolvesPost()) { | ||
| // check if requesting user is allowed to mark this answer message as resolving, i.e. if user is author or original message or at least tutor | ||
| mayMarkAnswerMessageAsResolvingElseThrow(existingAnswerMessage, user, course); | ||
| existingAnswerMessage.setResolvesPost(answerMessage.resolvesPost()); | ||
| // sets the message as resolved if there exists any resolving answer | ||
| existingAnswerMessage.getPost().setResolved(existingAnswerMessage.getPost().getAnswers().stream().anyMatch(AnswerPost::doesResolvePost)); | ||
| postRepository.save(existingAnswerMessage.getPost()); | ||
| resolutionChanged = true; | ||
| } | ||
| else { | ||
| // check if requesting user is allowed to update the content, i.e. if user is author of answer message or at least tutor | ||
|
|
@@ -251,9 +257,35 @@ public AnswerPost updateAnswerMessage(Long courseId, Long answerMessageId, Updat | |
| syncAnswerPostWithWeaviate(updatedAnswerMessage, conversation); | ||
|
|
||
| this.preparePostAndBroadcast(updatedAnswerMessage, course); | ||
|
|
||
| // Trigger B: the thread's resolution state changed, or the text an existing entry was built from | ||
| // did. Resolution changes fire in both directions so un-marking the last resolving answer retracts | ||
| // the entry instead of leaving it served as verified. Content edits re-ingest because the entry | ||
| // would otherwise keep serving the wording the author just corrected — the upsert is keyed on the | ||
| // thread, so re-sending it replaces the entry rather than adding a second one. | ||
| if (resolutionChanged || contributesToCourseMemory(updatedAnswerMessage)) { | ||
| try { | ||
| courseMemoryIngestionApi.ifPresent(api -> api.onThreadResolutionChanged(updatedAnswerMessage.getPost(), updatedAnswerMessage, user, course)); | ||
| } | ||
| catch (Exception e) { | ||
| log.error("Failed to update course memory after update of answer post {}", updatedAnswerMessage.getId(), e); | ||
| } | ||
| } | ||
| return updatedAnswerMessage; | ||
| } | ||
|
|
||
| /** | ||
| * Whether this answer is what a thread's Course Memory entry would have been built from, i.e. whether | ||
| * changing or removing it has to be reflected there. True for an answer that resolves the thread and | ||
| * for a verified Iris answer, which the verification trigger stores in its own right. | ||
| * | ||
| * @param answerMessage the answer to check | ||
| * @return {@code true} if the thread's entry depends on this answer | ||
| */ | ||
| private boolean contributesToCourseMemory(AnswerPost answerMessage) { | ||
| return Boolean.TRUE.equals(answerMessage.doesResolvePost()) || (answerMessage.getAuthor() != null && answerMessage.getAuthor().isBot() && answerMessage.isVerified()); | ||
| } | ||
|
|
||
| private Conversation mayUpdateOrDeleteAnswerMessageElseThrow(AnswerPost existingAnswerPost, User user) { | ||
| boolean userIsAuthor = existingAnswerPost.getAuthor().getId().equals(user.getId()); | ||
| Conversation conversation = existingAnswerPost.getPost().getConversation(); | ||
|
|
@@ -296,6 +328,11 @@ public void deleteAnswerMessageById(Long courseId, Long answerMessageId) { | |
| } | ||
| ensureConversationBelongsToCourseElseThrow(conversation, courseId); | ||
|
|
||
| // An answer that resolved the thread or that was a verified Iris answer is what the thread's | ||
| // Course Memory entry was built from, so its removal has to be reflected there too. Evaluated | ||
| // before the delete, while the answer is still loaded. | ||
| boolean contributedToCourseMemory = contributesToCourseMemory(answerMessage); | ||
|
|
||
| // we need to explicitly remove the answer post from the answers of the broadcast post to share up-to-date information | ||
| Post updatedMessage = answerMessage.getPost(); | ||
| updatedMessage.removeAnswerPost(answerMessage); | ||
|
|
@@ -314,6 +351,17 @@ public void deleteAnswerMessageById(Long courseId, Long answerMessageId) { | |
| savedPostRepository.deleteAll(savedPosts); | ||
|
|
||
| broadcastForPost(updatedMessage, MetisCrudAction.UPDATE, course.getId(), null); | ||
|
|
||
| // Re-ingest from whatever verified answer survives, or delete the entry if none does. Runs after | ||
| // the delete is committed so the re-fetched thread no longer contains the removed answer. | ||
| if (contributedToCourseMemory) { | ||
| try { | ||
| courseMemoryIngestionApi.ifPresent(api -> api.onThreadResolutionChanged(updatedMessage, null, user, course)); | ||
| } | ||
| catch (Exception e) { | ||
| log.error("Failed to update course memory after deletion of answer post {}", answerMessageId, e); | ||
| } | ||
| } | ||
|
Comment on lines
+354
to
+364
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Description: Inspect the eager thread query and any transaction annotations on the answer deletion path.
rg -n -C6 'findByPostIdsWithEagerRelationships' --type=java
rg -n -C3 'Transactional' src/main/java/de/tum/cit/aet/artemis/communication/service/AnswerMessageService.java src/main/java/de/tum/cit/aet/artemis/communication/web/ --type=javaRepository: ls1intum/Artemis Length of output: 154 🏁 Script executed: #!/bin/bash
set -u
echo "Repository files matching relevant names:"
fd -i 'AnswerMessageService|ConversationMessageRepository|CourseMemoryIngestionService|CourseMemoryIngestionApi' .
echo
echo "Find Java files by broad search if fd unavailable:"
git ls-files '*.java' | rg 'AnswerMessageService|ConversationMessageRepository|CourseMemoryIngestion|AnswerPost|ConversationMessage' | sed -n '1,200p'Repository: ls1intum/Artemis Length of output: 2617 🏁 Script executed: #!/bin/bash
set -u
echo "AnswerMessageService outline:"
ast-grep outline src/main/java/de/tum/cit/aet/artemis/communication/service/AnswerMessageService.java --view expanded | sed -n '1,220p' || true
echo
echo "ConversationMessageRepository outline:"
ast-grep outline src/main/java/de/tum/cit/aet/artemis/communication/repository/ConversationMessageRepository.java --view expanded | sed -n '1,220p' || true
echo
echo "CourseMemoryIngestionService outline:"
ast-grep outline src/main/java/de/tum/cit/aet/artemis/iris/service/CourseMemoryIngestionService.java --view expanded | sed -n '1,240p' || true
echo
echo "CourseMemoryIngestionApi outline:"
ast-grep outline src/main/java/de/tum/cit/aet/artemis/iris/api/CourseMemoryIngestionApi.java --view expanded | sed -n '1,180p' || trueRepository: ls1intum/Artemis Length of output: 4911 🏁 Script executed: #!/bin/bash
set -u
echo "AnswerMessageService relevant sections:"
sed -n '260,355p' src/main/java/de/tum/cit/aet/artemis/communication/service/AnswerMessageService.java | cat -n
echo
echo "AnswerMessageService imports"
sed -n '1,90p' src/main/java/de/tum/cit/aet/artemis/communication/service/AnswerMessageService.java | cat -n
echo
echo "ConversationMessageRepository relevant sections:"
sed -n '90,145p' src/main/java/de/tum/cit/aet/artemis/communication/repository/ConversationMessageRepository.java | cat -n
echo
echo "CourseMemoryIngestionService relevant sections:"
sed -n '120,350p' src/main/java/de/tum/cit/aet/artemis/iris/service/CourseMemoryIngestionService.java | cat -n
echo
echo "CourseMemoryIngestionApi relevant sections:"
sed -n '1,75p' src/main/java/de/tum/cit/aet/artemis/iris/api/CourseMemoryIngestionApi.java | cat -n
echo
echo "Search repository symbols"
rg -n -C4 'AnswerPostRepository|AnswerPost|AnswerMessage|conversationMessageRepository|deleteAnswerMessageById|onThreadResolutionChanged|fetchThread|selectedAnchor|selectAnchor|deleteThreadMemory|ingest' src/main/java/de/tum/cit/aet/artemis/communication/repository src/main/java/de/tum/cit/aet/artemis/communication/service src/main/java/de/tum/cit/aet/artemis/communication/web src/main/java/de/tum/cit/aet/artemis/iris/service src/main/java/de/tum/cit/aet/artemis/iris/api --type=javaRepository: ls1intum/Artemis Length of output: 50373 🏁 Script executed: #!/bin/bash
set -u
echo "CourseMemoryIngestionService.onThreadResolutionChanged/fetchThread/selectAnchor:"
sed -n '140,215p' src/main/java/de/tum/cit/aet/artemis/iris/service/CourseMemoryIngestionService.java | cat -n
echo
echo "CourseMemoryIngestionService.fetchThread implementation:"
sed -n '315,326p' src/main/java/de/tum/cit/aet/artemis/iris/service/CourseMemoryIngestionService.java | cat -n
echo
echo "CourseMemoryIngestionService.relevant sections (fetchThread plus helpers):"
sed -n '315,345p' src/main/java/de/tum/cit/aet/artemis/iris/service/CourseMemoryIngestionService.java | cat -n
echo
echo "CourseMemoryIngestionService imports"
sed -n '1,80p' src/main/java/de/tum/cit/aet/artemis/iris/service/CourseMemoryIngestionService.java | cat -n
echo
echo "Exact symbols across Java files:"
rg -n '\bfetchThread\b|\bselectAnchor\b|\bbuildThread\b|\bhandleResolutionChange\b|\bonThreadResolutionChanged\b|\bdeleteAnswerMessageById\b|\bcontributedToCourseMemory\b|\bcontainsAnswerPost\b|\bloade' src/main/java/de/tum/cit/aet/artemis --type=javaRepository: ls1intum/Artemis Length of output: 50372 🏁 Script executed: #!/bin/bash
set -u
echo "ConversationMessageRepository lines 99-130:"
sed -n '99,130p' src/main/java/de/tum/cit/aet/artemis/communication/repository/ConversationMessageRepository.java | cat -n
echo
echo "CourseMemoryIngestionService.handleResolutionChange/selectAnchor/buildThread:"
sed -n '140,180p' src/main/java/de/tum/cit/aet/artemis/iris/service/CourseMemoryIngestionService.java | cat -n
echo
echo "findAnswerMessageByIdElseThrow implementations:"
rg -n -C6 'findAnswerMessageByIdElseThrow|findAnswerMessage.*PessimisticWriteLock|findByIdIn' src/main/java/de/tum/cit/aet/artemis/communication/repository src/main/java/de/tum/cit/aet/artemis/communication/service --type=java
echo
echo "Delete path transaction annotations in focused files:"
rg -n '`@Transaction`|Transactional|runInTransaction|execute|`@Async`|`@Async`' src/main/java/de/tum/cit/aet/artemis/communication/repository src/main/java/de/tum/cit/aet/artemis/communication/service/AnswerMessageService.java src/main/java/de/tum/cit/aet/artemis/communication/web --type=javaRepository: ls1intum/Artemis Length of output: 23582 🏁 Script executed: #!/bin/bash
set -u
echo "AnswerMessageService imports and bean declarations:"
sed -n '1,110p' src/main/java/de/tum/cit/aet/artemis/communication/service/AnswerMessageService.java | cat -n
echo
echo "CourseMemoryIngestionService call path annotations:"
rg -n 'public |private |`@Transactional`|`@Async`|try \{|catch \(' src/main/java/de/tum/cit/aet/artemis/communication/service/AnswerMessageService.java:295 src/main/java/de/tum/cit/aet/artemis/communication/service/AnswerMessageService.java:340 src/main/java/de/tum/cit/aet/artemis/iris/service/CourseMemoryIngestionService.java:145 --type=java || true
echo
echo "AnswerPostRepository imports and first transactional declarations:"
sed -n '1,60p' src/main/java/de/tum/cit/aet/artemis/communication/repository/AnswerPostRepository.java | cat -n
echo
echo "ConversationMessageRepository transactional declarations:"
sed -n '1,115p' src/main/java/de/tum/cit/aet/artemis/communication/repository/ConversationMessageRepository.java | cat -nRepository: ls1intum/Artemis Length of output: 18162 🌐 Web query:
💡 Result: In Spring Data JPA, repository methods follow specific transactional behaviors depending on whether they are standard CRUD methods or custom query methods [1][2]. Default Transactional Behavior By default, Spring Data JPA repositories are transactional, governed by the SimpleJpaRepository implementation class [1][3]. - CRUD Methods: Standard methods inherited from CrudRepository (e.g., save, delete, findAll) are annotated with Citations:
🌐 Web query:
💡 Result: In Hibernate, when you remove an element from a collection that is part of a persistence context, the behavior depends on the collection mapping and whether the association is bidirectional or unidirectional [1][2]. Hibernate tracks changes to these collections by wrapping them in a PersistentCollection [3]. During the flush process, Hibernate synchronizes the in-memory state of these collections with the database [4]. For many-to-many or one-to-many associations, removing an item from the collection typically results in Hibernate executing a DELETE statement for the association row in the link table (for many-to-many) or updating the foreign key column to null (for one-to-many), depending on the specific mapping configuration [5][6][7]. If you remove all elements from a collection, Hibernate may clear the entire association by deleting all corresponding rows in the database [1][8]. Key considerations for managing this behavior include: 1. Bidirectional Associations: It is mandatory to synchronize both sides of a bidirectional relationship [9]. If you only remove an entity from the collection on the parent side, the database state may not update correctly if the inverse side (the child) still holds a reference to the parent [9]. Always provide helper methods (e.g., add/remove) on your entities to ensure both sides of the association are kept in sync [9]. 2. Performance: Modifying a collection—especially lists or bags—can sometimes be inefficient if Hibernate determines it must "recreate" the collection state by deleting and re-inserting elements, rather than issuing specific delete or update statements [1][8]. Using an Citations:
🌐 Web query:
💡 Result: In Hibernate, the first-level cache (the Persistence Context) acts as a transactional write-behind buffer that stores managed entities [1][2]. When you modify an entity—including its associations—within the Persistence Context, those changes are held in memory and are not immediately synchronized with the database [1][3]. When a flush operation occurs, Hibernate performs "dirty checking" to synchronize the in-memory state of managed entities with the underlying database [2]. During this process, Hibernate compares the current state of the managed entities in the first-level cache against the "loaded state" (a snapshot taken when the entity was originally fetched) [1]. If discrepancies are detected, Hibernate enqueues the necessary SQL statements (INSERT, UPDATE, DELETE) to align the database with the current state of the Persistence Context [4][3]. Regarding modified associations specifically: 1. Consistency and Synchronization: Hibernate does not automatically manage the synchronization of both sides of a bidirectional association [5][6]. It is the developer's responsibility to ensure that both sides of an association are updated in memory before the flush occurs [5]. Failure to manually synchronize both ends of a bidirectional relationship can lead to inconsistent state in the Persistence Context, which may result in data integrity issues or incorrect SQL being generated during the flush [5]. 2. Flush Behavior: The flush operation ensures that all pending state transitions—including association changes—are reflected in the database [3]. Because Hibernate uses a write-behind strategy, the actual SQL execution is deferred and optimized [2][3]. It is important to note that the flush order is specific (e.g., entity inserts generally precede collection updates or removals) [4]. 3. Data Visibility: Flushed changes are visible only within the current database transaction [3]. They remain pending in the database until the transaction is committed [3]. To ensure reliable behavior, always use helper methods to synchronize both sides of a bidirectional association immediately after modifying them in your application code [5][6]. Do not rely on Hibernate to reconcile asymmetric bidirectional associations during the flush process [5]. Citations:
Use a separate transaction context for the re-fetch.
🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| /** | ||
|
|
@@ -384,6 +432,15 @@ public AnswerPost verifyAnswerMessage(Long courseId, Long answerMessageId, Verif | |
|
|
||
| sendMentionNotificationForAnswerMessage(course, verificationResult.conversation(), verificationResult.answerMessage(), verificationResult.mentionedUsers()); | ||
| this.preparePostAndBroadcast(verificationResult.answerMessage(), course); | ||
|
|
||
| // Trigger A: a tutor approved (IRIS_AUTO) or edited (IRIS_CORRECTED) an Iris draft -> ingest into Course Memory | ||
| boolean edited = verifyDto != null && verifyDto.content() != null && !verifyDto.content().isBlank(); | ||
| try { | ||
| courseMemoryIngestionApi.ifPresent(api -> api.onAnswerVerified(verificationResult.answerMessage(), edited, user, course)); | ||
| } | ||
| catch (Exception e) { | ||
| log.error("Failed to ingest verified answer post {} into course memory", verificationResult.answerMessage().getId(), e); | ||
| } | ||
| return verificationResult.answerMessage(); | ||
| } | ||
|
|
||
|
|
||
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.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Refresh edited verified Iris answers through the verification path.
If an authorized user edits a verified Iris answer that does not resolve the post,
contributesToCourseMemoryis true. The resolution callback then selects that answer andCourseMemoryIngestionServiceskips it because Trigger A owns it. The stored entry keeps the old text.Add a dedicated update path for verified Iris answers that preserves the edited text and provenance.
🤖 Prompt for AI Agents