-
Notifications
You must be signed in to change notification settings - Fork 304
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
Iris
: Add lecture transcription storage
#10176
base: develop
Are you sure you want to change the base?
Changes from all commits
3eeeb99
6335ee1
a357749
13226bc
3f9a557
151abde
750a179
cad2db6
d6ee26e
4ed75e2
ffb02a6
ca47710
7e84063
1b24721
ba2dd37
a22949a
04b63e9
57427ed
b733370
a31455f
256d5c9
6f3c804
26ba567
1a820ed
01320d3
627ced0
90344df
8e9d776
0be3c13
3c65bf9
d72c175
afde4d2
1d3c6b2
5191ef0
079d088
e2c552c
53dcba7
19edc8d
4f7f425
62135c7
94e6540
0c3f5cf
08c6821
b3e2f8a
0bc450a
0546068
2f35db8
1e2d355
21797e7
2c05f92
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 |
---|---|---|
|
@@ -35,10 +35,14 @@ | |
import de.tum.cit.aet.artemis.iris.service.pyris.dto.lectureingestionwebhook.PyrisLectureUnitWebhookDTO; | ||
import de.tum.cit.aet.artemis.iris.service.pyris.dto.lectureingestionwebhook.PyrisWebhookLectureDeletionExecutionDTO; | ||
import de.tum.cit.aet.artemis.iris.service.pyris.dto.lectureingestionwebhook.PyrisWebhookLectureIngestionExecutionDTO; | ||
import de.tum.cit.aet.artemis.iris.service.pyris.dto.transcriptionIngestion.PyrisTranscriptionIngestionWebhookDTO; | ||
import de.tum.cit.aet.artemis.iris.service.pyris.dto.transcriptionIngestion.PyrisWebhookTranscriptionDeletionExecutionDTO; | ||
import de.tum.cit.aet.artemis.iris.service.pyris.dto.transcriptionIngestion.PyrisWebhookTranscriptionIngestionExecutionDTO; | ||
import de.tum.cit.aet.artemis.iris.service.settings.IrisSettingsService; | ||
import de.tum.cit.aet.artemis.lecture.domain.AttachmentType; | ||
import de.tum.cit.aet.artemis.lecture.domain.AttachmentUnit; | ||
import de.tum.cit.aet.artemis.lecture.domain.Lecture; | ||
import de.tum.cit.aet.artemis.lecture.domain.LectureTranscription; | ||
import de.tum.cit.aet.artemis.lecture.domain.LectureUnit; | ||
import de.tum.cit.aet.artemis.lecture.repository.LectureRepository; | ||
import de.tum.cit.aet.artemis.lecture.repository.LectureUnitRepository; | ||
|
@@ -74,6 +78,85 @@ public PyrisWebhookService(PyrisConnectorService pyrisConnectorService, PyrisJob | |
this.lectureRepository = lectureRepository; | ||
} | ||
|
||
/** | ||
* adds the transcription to the vector database in Pyris | ||
* | ||
* @param transcriptions The transcription that got Updated | ||
* @param course The course of the transcriptions | ||
* @param lecture The lecture of the transcriptions | ||
* @param lectureUnit The lecture unit of the transcriptions | ||
* @return jobToken if the job was created else null | ||
*/ | ||
public String addTranscriptionsToPyrisDB(Set<LectureTranscription> transcriptions, Course course, Lecture lecture, LectureUnit lectureUnit) { | ||
if (transcriptions == null || transcriptions.isEmpty()) { | ||
throw new IllegalArgumentException("Transcriptions cannot be empty"); | ||
} | ||
|
||
if (!lectureIngestionEnabled(course)) { | ||
return null; | ||
} | ||
|
||
transcriptions.forEach(transcription -> { | ||
if (transcription.getLecture() == null) { | ||
throw new IllegalArgumentException("Transcription must be associated with a lecture"); | ||
} | ||
else if (!transcription.getLecture().equals(lecture)) { | ||
throw new IllegalArgumentException("All transcriptions must be associated with the same lecture"); | ||
} | ||
}); | ||
isabellagessl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
List<PyrisTranscriptionIngestionWebhookDTO> pyrisTranscriptionIngestionWebhookDTOs = transcriptions.stream() | ||
.map(transcription -> new PyrisTranscriptionIngestionWebhookDTO(transcription, lecture.getId(), lecture.getTitle(), course.getId(), course.getTitle(), | ||
course.getDescription())) | ||
.toList(); | ||
|
||
return executeTranscriptionAdditionWebhook(pyrisTranscriptionIngestionWebhookDTOs, course, lecture, lectureUnit); | ||
} | ||
|
||
/** | ||
* adds the lecture transcription into the vector database of Pyris | ||
* | ||
* @param toUpdateTranscriptions The transcription that are going to be Updated | ||
* @return jobToken if the job was created | ||
*/ | ||
private String executeTranscriptionAdditionWebhook(List<PyrisTranscriptionIngestionWebhookDTO> toUpdateTranscriptions, Course course, Lecture lecture, | ||
isabellagessl marked this conversation as resolved.
Show resolved
Hide resolved
|
||
LectureUnit lectureUnit) { | ||
String jobToken = pyrisJobService.addTranscriptionIngestionWebhookJob(course.getId(), lecture.getId(), lectureUnit.getId()); | ||
PyrisPipelineExecutionSettingsDTO settingsDTO = new PyrisPipelineExecutionSettingsDTO(jobToken, List.of(), artemisBaseUrl); | ||
PyrisWebhookTranscriptionIngestionExecutionDTO executionDTO = new PyrisWebhookTranscriptionIngestionExecutionDTO(toUpdateTranscriptions, lectureUnit.getId(), settingsDTO, | ||
List.of()); | ||
pyrisConnectorService.executeTranscriptionAdditionWebhook("fullIngestion", executionDTO); | ||
return jobToken; | ||
} | ||
|
||
/** | ||
* delete the lecture transcription in pyris | ||
* | ||
* @param lectureTranscription The lecture transcription that gets erased | ||
* @return jobToken if the job was created | ||
*/ | ||
public String deleteLectureTranscription(LectureTranscription lectureTranscription) { | ||
Lecture lecture = lectureTranscription.getLecture(); | ||
Course course = lecture.getCourse(); | ||
return executeLectureTranscriptionDeletionWebhook( | ||
new PyrisTranscriptionIngestionWebhookDTO(lectureTranscription, lecture.getId(), lecture.getTitle(), course.getId(), course.getTitle(), course.getDescription())); | ||
} | ||
|
||
/** | ||
* executes the faq deletion webhook to delete faq from the vector database on pyris | ||
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. update this one please |
||
* | ||
* @param toUpdateLectureTranscription The lecture transcription that got Updated as webhook DTO | ||
* @return jobToken if the job was created else null | ||
*/ | ||
private String executeLectureTranscriptionDeletionWebhook(PyrisTranscriptionIngestionWebhookDTO toUpdateLectureTranscription) { | ||
String jobToken = pyrisJobService.addTranscriptionIngestionWebhookJob(0, 0, 0); | ||
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. are the params intended ? |
||
PyrisPipelineExecutionSettingsDTO settingsDTO = new PyrisPipelineExecutionSettingsDTO(jobToken, List.of(), artemisBaseUrl); | ||
PyrisWebhookTranscriptionDeletionExecutionDTO executionDTO = new PyrisWebhookTranscriptionDeletionExecutionDTO(toUpdateLectureTranscription, settingsDTO, List.of()); | ||
pyrisConnectorService.executeLectureTranscriptionDeletionWebhook(executionDTO); | ||
|
||
return jobToken; | ||
} | ||
Comment on lines
+151
to
+158
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. Use actual IDs in job token generation for accurate tracking In String jobToken = pyrisJobService.addTranscriptionIngestionWebhookJob(0, 0, 0); Consider using the actual Apply this diff to correct the job token generation: private String executeLectureTranscriptionDeletionWebhook(PyrisTranscriptionIngestionWebhookDTO toUpdateLectureTranscription) {
- String jobToken = pyrisJobService.addTranscriptionIngestionWebhookJob(0, 0, 0);
+ String jobToken = pyrisJobService.addTranscriptionIngestionWebhookJob(
+ toUpdateLectureTranscription.courseId(),
+ toUpdateLectureTranscription.lectureId(),
+ toUpdateLectureTranscription.lectureUnitId()
+ );
PyrisPipelineExecutionSettingsDTO settingsDTO = new PyrisPipelineExecutionSettingsDTO(jobToken, List.of(), artemisBaseUrl);
PyrisWebhookTranscriptionDeletionExecutionDTO executionDTO =
new PyrisWebhookTranscriptionDeletionExecutionDTO(toUpdateLectureTranscription, settingsDTO, List.of());
pyrisConnectorService.executeLectureTranscriptionDeletionWebhook(executionDTO);
return jobToken;
} |
||
|
||
private boolean lectureIngestionEnabled(Course course) { | ||
return irisSettingsService.getRawIrisSettingsFor(course).getIrisLectureIngestionSettings() != null | ||
&& irisSettingsService.getRawIrisSettingsFor(course).getIrisLectureIngestionSettings().isEnabled(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package de.tum.cit.aet.artemis.iris.service.pyris.dto.transcriptionIngestion; | ||
|
||
import com.fasterxml.jackson.annotation.JsonInclude; | ||
|
||
import de.tum.cit.aet.artemis.lecture.domain.LectureTranscription; | ||
|
||
/** | ||
* Represents a webhook data transfer object for lecture transcriptions on lecture unit level in the Pyris system. | ||
* This DTO is used to encapsulate the information related to updates of lecture units, | ||
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. The second part seems to be wrong here |
||
* providing necessary details such as lecture and course identifiers, names, and descriptions. | ||
*/ | ||
@JsonInclude(JsonInclude.Include.NON_EMPTY) | ||
|
||
public record PyrisTranscriptionIngestionWebhookDTO(LectureTranscription transcription, long lectureId, String lectureName, long courseId, String courseName, | ||
String courseDescription) { | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,13 @@ | ||||||||||||
package de.tum.cit.aet.artemis.iris.service.pyris.dto.transcriptionIngestion; | ||||||||||||
|
||||||||||||
import java.util.List; | ||||||||||||
|
||||||||||||
import com.fasterxml.jackson.annotation.JsonInclude; | ||||||||||||
|
||||||||||||
import de.tum.cit.aet.artemis.iris.service.pyris.dto.PyrisPipelineExecutionSettingsDTO; | ||||||||||||
import de.tum.cit.aet.artemis.iris.service.pyris.dto.status.PyrisStageDTO; | ||||||||||||
|
||||||||||||
@JsonInclude(JsonInclude.Include.NON_EMPTY) | ||||||||||||
public record PyrisWebhookTranscriptionDeletionExecutionDTO(PyrisTranscriptionIngestionWebhookDTO lectureTranscriptionDTO, PyrisPipelineExecutionSettingsDTO settings, | ||||||||||||
List<PyrisStageDTO> initalStages) { | ||||||||||||
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. Fix typo in parameter name There's a typo in the parameter name Apply this diff to correct the parameter name: public record PyrisWebhookTranscriptionDeletionExecutionDTO(PyrisTranscriptionIngestionWebhookDTO lectureTranscriptionDTO, PyrisPipelineExecutionSettingsDTO settings,
- List<PyrisStageDTO> initalStages) {
+ List<PyrisStageDTO> initialStages) { Ensure to update any usages of this parameter throughout the codebase. 📝 Committable suggestion
Suggested change
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. coderabbit is right, typo`? |
||||||||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package de.tum.cit.aet.artemis.iris.service.pyris.dto.transcriptionIngestion; | ||
|
||
import java.util.List; | ||
|
||
import com.fasterxml.jackson.annotation.JsonInclude; | ||
|
||
import de.tum.cit.aet.artemis.iris.service.pyris.dto.PyrisPipelineExecutionSettingsDTO; | ||
import de.tum.cit.aet.artemis.iris.service.pyris.dto.status.PyrisStageDTO; | ||
|
||
@JsonInclude(JsonInclude.Include.NON_EMPTY) | ||
public record PyrisWebhookTranscriptionIngestionExecutionDTO(List<PyrisTranscriptionIngestionWebhookDTO> transcriptions, Long lectureUnitId, | ||
PyrisPipelineExecutionSettingsDTO settings, List<PyrisStageDTO> initialStages) { | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
package de.tum.cit.aet.artemis.iris.service.pyris.job; | ||
|
||
import de.tum.cit.aet.artemis.core.domain.Course; | ||
import de.tum.cit.aet.artemis.exercise.domain.Exercise; | ||
|
||
/** | ||
* An implementation of a PyrisJob for Transcription Ingestion in Pyris. | ||
* This job is used to reference the details of then Ingestion when Pyris sends a status update. | ||
*/ | ||
public record TranscriptionIngestionWebhookJob(String jobId, long courseId, long lectureId, long lectureUnitId) implements PyrisJob { | ||
|
||
@Override | ||
public boolean canAccess(Course course) { | ||
return false; | ||
} | ||
|
||
@Override | ||
public boolean canAccess(Exercise exercise) { | ||
return false; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,93 @@ | ||
package de.tum.cit.aet.artemis.lecture.domain; | ||
|
||
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
import jakarta.persistence.CascadeType; | ||
import jakarta.persistence.Entity; | ||
import jakarta.persistence.FetchType; | ||
import jakarta.persistence.JoinColumn; | ||
import jakarta.persistence.ManyToOne; | ||
import jakarta.persistence.OneToMany; | ||
import jakarta.persistence.OrderBy; | ||
import jakarta.persistence.Table; | ||
import jakarta.validation.constraints.Size; | ||
|
||
import org.hibernate.annotations.Cache; | ||
import org.hibernate.annotations.CacheConcurrencyStrategy; | ||
|
||
import com.fasterxml.jackson.annotation.JsonIgnore; | ||
|
||
import de.tum.cit.aet.artemis.core.domain.DomainObject; | ||
|
||
@Entity | ||
@Table(name = "lecture_transcription") | ||
public class LectureTranscription extends DomainObject { | ||
|
||
@ManyToOne | ||
@JoinColumn(name = "lecture_id") | ||
@Cache(usage = CacheConcurrencyStrategy.NONSTRICT_READ_WRITE) | ||
@JsonIgnore | ||
private Lecture lecture; | ||
|
||
@Size(min = 2, max = 2, message = "Language must be exactly 2 characters long") | ||
private String language; | ||
|
||
@OneToMany(cascade = CascadeType.ALL, fetch = FetchType.LAZY, orphanRemoval = true) | ||
@OrderBy("startTime asc") | ||
@JoinColumn(name = "transcription_id") | ||
private List<LectureTranscriptionSegment> segments = new ArrayList<>(); | ||
|
||
@ManyToOne | ||
@JoinColumn(name = "lecture_unit_id") | ||
@Cache(usage = CacheConcurrencyStrategy.NONSTRICT_READ_WRITE) | ||
@JsonIgnore | ||
private LectureUnit lectureUnit; | ||
|
||
public LectureTranscription() { | ||
} | ||
|
||
public LectureTranscription(Lecture lecture, String language, List<LectureTranscriptionSegment> segments, LectureUnit lectureUnit) { | ||
this.lecture = lecture; | ||
this.language = language; | ||
this.segments = segments; | ||
this.lectureUnit = lectureUnit; | ||
} | ||
|
||
public Lecture getLecture() { | ||
return lecture; | ||
} | ||
|
||
public void setLecture(Lecture lecture) { | ||
this.lecture = lecture; | ||
} | ||
|
||
public String getLanguage() { | ||
return language; | ||
} | ||
|
||
public void setLanguage(String language) { | ||
this.language = language; | ||
} | ||
|
||
public List<LectureTranscriptionSegment> getSegments() { | ||
return segments; | ||
} | ||
|
||
public void setSegments(List<LectureTranscriptionSegment> segments) { | ||
this.segments = segments; | ||
} | ||
|
||
public LectureUnit getLectureUnit() { | ||
return lectureUnit; | ||
} | ||
|
||
public void setLectureUnit(LectureUnit lectureUnit) { | ||
this.lectureUnit = lectureUnit; | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return "Transcription [language=" + language + ", segments=" + segments + "]"; | ||
} | ||
} |
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.
Correct the inconsistency in the method documentation
The Javadoc comment for
executeLectureTranscriptionDeletionWebhook
is inconsistent and confusing. It mentions both adding and deleting lecture transcriptions. Please update the documentation to accurately reflect the method's purpose of deleting lecture transcriptions.Apply this diff to correct the Javadoc comment:
📝 Committable suggestion