Iris: Match citations to versions of slides and videos - #13441
Conversation
End-to-End Test Results
❌ Failed Tests
Test Strategy: Running all tests (configuration or infrastructure changes detected) Overall: ❌ E2E: real (non-flaky) test failure 🔗 Workflow Run · 📊 Test Report |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughIris citations now include attachment and transcription versions. Backend processing tracks transcription revisions and stamps chat results. The frontend parses pinned versions, validates current material versions, and handles stale or unavailable lecture content. Deep-link navigation reports missing lecture units. ChangesCitation version tracking and backend stamping
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant IrisChatSession
participant IrisCitationService
participant LectureUnitRepository
participant IrisCitationTextComponent
participant IrisCitationMaterialVersionService
participant LectureUnitResource
IrisChatSession->>IrisCitationService: stampCitationVersions(text)
IrisCitationService->>LectureUnitRepository: findIngestedVersionsByIds(ids)
LectureUnitRepository-->>IrisCitationService: ingested material versions
IrisCitationService-->>IrisChatSession: stamped citation text
IrisCitationTextComponent->>IrisCitationMaterialVersionService: getMaterialVersions(lectureUnitId)
IrisCitationMaterialVersionService->>LectureUnitResource: GET material-versions
LectureUnitResource-->>IrisCitationTextComponent: current material versions
IrisCitationTextComponent->>IrisCitationTextComponent: validate versions before routing
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/de/tum/cit/aet/artemis/iris/service/IrisCitationService.java`:
- Line 195: Update the citation stamping lookup around ingestedVersions and
extractEntityIds to handle oversized matched entity IDs before Long.parseLong;
when parsing fails or the ID cannot fit in a long, preserve and return the
original citation while continuing valid citations. Add a regression test
covering mixed valid and oversized IDs.
In
`@src/main/java/de/tum/cit/aet/artemis/lecture/service/ProcessingStateCallbackService.java`:
- Around line 465-468: The canonical serialization built in the segment loop
must be unambiguous when any field, especially
LectureTranscriptionSegment.text(), contains separators or line breaks. Replace
the delimiter-only appends in the canonical hashing flow with length-delimited
or structured encoding for every segment field, and add test coverage for
separator and newline content producing distinct hashes.
In
`@src/main/webapp/app/iris/overview/citation-text/iris-citation-text.component.ts`:
- Around line 311-316: Update the pinned-version checks in the citation
attribute-building logic around videoVersion and attachmentVersion to test
explicitly for undefined, preserving version 0 and selecting the correct pinned
kind. Add a regression test covering a video or attachment version of 0 and
verifying both pinned attributes are emitted.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e1aeb787-dc85-4a97-b1a4-01f307fe8812
⛔ Files ignored due to path filters (2)
src/main/resources/config/liquibase/changelog/20260729121844_changelog.xmlis excluded by!**/*.xmlsrc/main/resources/config/liquibase/master.xmlis excluded by!**/*.xml
📒 Files selected for processing (19)
src/main/java/de/tum/cit/aet/artemis/iris/service/IrisCitationService.javasrc/main/java/de/tum/cit/aet/artemis/iris/service/session/AbstractIrisChatSessionService.javasrc/main/java/de/tum/cit/aet/artemis/lecture/api/LectureUnitRepositoryApi.javasrc/main/java/de/tum/cit/aet/artemis/lecture/domain/LectureUnitProcessingState.javasrc/main/java/de/tum/cit/aet/artemis/lecture/dto/LectureUnitIngestedVersionsDTO.javasrc/main/java/de/tum/cit/aet/artemis/lecture/dto/LectureUnitMaterialVersionsDTO.javasrc/main/java/de/tum/cit/aet/artemis/lecture/repository/LectureUnitRepository.javasrc/main/java/de/tum/cit/aet/artemis/lecture/service/ProcessingStateCallbackService.javasrc/main/java/de/tum/cit/aet/artemis/lecture/web/LectureUnitResource.javasrc/main/webapp/app/iris/overview/citation-text/iris-citation-material-version.service.tssrc/main/webapp/app/iris/overview/citation-text/iris-citation-text.component.spec.tssrc/main/webapp/app/iris/overview/citation-text/iris-citation-text.component.tssrc/main/webapp/app/iris/overview/citation-text/iris-citation-text.model.tssrc/main/webapp/app/iris/overview/citation-text/iris-citation-text.util.tssrc/main/webapp/i18n/de/iris.jsonsrc/main/webapp/i18n/en/iris.jsonsrc/test/java/de/tum/cit/aet/artemis/iris/service/IrisCitationServiceTest.javasrc/test/java/de/tum/cit/aet/artemis/lecture/LectureUnitIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/lecture/service/ProcessingStateCallbackServiceClassificationTest.java
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@vivienxr Changes are requested because the new transcription revision state is not preserved by the existing manual-retry path; the inline comment describes the resulting stale-citation failure. The unresolved CodeRabbit findings on oversized citation IDs and ambiguous transcription hashing are also reproducible on this head and still need fixes. The captured required-CI aggregate is failing, while the replacement server and client test jobs were still in progress, so this snapshot does not provide a clean required-CI result.
| * <p> | ||
| * Iris citations pin this value so that a cited video timestamp can be checked against the transcription it was generated from. | ||
| * It lives here rather than on {@link LectureTranscription} because that row is deleted and recreated when the video changes, | ||
| * which would reset a per-row counter; this row survives. |
There was a problem hiding this comment.
@vivienxr [medium] This row does not always survive: LectureContentProcessingService.retryProcessing deletes a failed processing state and creates a fresh one, discarding both the transcription version and its hash. An ingestion failure followed by manual retry therefore loses the current revision; after another video change the counter can restart at 1, causing an old version-1 citation to be treated as current and jump to an unrelated timestamp. Preserve both fields when replacing the failed state (or reuse the existing row), and cover the retry sequence with a regression test.
🤖 Prompt for AI agents
In src/main/java/de/tum/cit/aet/artemis/lecture/domain/LectureUnitProcessingState.java, the supposedly monotonic transcription version and content hash are lost when manual retry replaces the processing-state row, allowing version reuse and stale citations to appear current. Preserve both values across manual retries or reuse the existing row, and add a regression test covering failure, retry, and a later transcription change.
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@vivienxr The previous retry-state, checkpoint-versioning, citation-format, and lecture-transition defects are resolved on this head. Two correctness gaps remain: pending transcription rows can expose an old version as current, and verified video citations can still navigate to an independently stale PDF page.
| if (timestamp) { | ||
| queryParams.timestamp = timestamp; | ||
| } | ||
| if (page) { |
There was a problem hiding this comment.
@vivienxr [medium] A video marker can carry both a timestamp and its companion page, but the server intentionally pins only the transcription version. Once that version matches, this method copies both coordinates; if the PDF changed independently, the PDF viewer still jumps to the now-unverified page alongside the valid video timestamp. For stamped citations, include only the coordinate belonging to the pinned material kind (or validate both materials), while preserving legacy behavior for unpinned citations.
🤖 Prompt for AI agents
In src/main/webapp/app/iris/overview/citation-text/iris-citation-text.component.ts, verified video citations also forward an unverified companion PDF page, allowing navigation to a stale slide when only the transcription version matches. Pass the pinned kind into navigation and include only the timestamp for video citations or only the page for attachment citations, preserving legacy unpinned navigation, and add a regression test.
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@vivienxr The latest commit resolves both remaining correctness issues: material-version lookup now excludes pending transcriptions, and verified video citations forward only the coordinate tied to the checked material. The surrounding lifecycle, parsing, authorization, migration, and regression coverage are consistent with the intended behavior. No required checks were configured in the captured CI snapshot, and no completed relevant check failed.
…f-slides-and-videos
…f-slides-and-videos
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/webapp/app/iris/overview/citation-text/iris-citation-text.util.ts (1)
238-246: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse an arrow function for
parseTrailingVersion.Convert the new helper to an arrow function without changing its behavior.
Proposed refactor
-function parseTrailingVersion(parts: string[]): IrisCitationVersion | undefined { +const parseTrailingVersion = (parts: string[]): IrisCitationVersion | undefined => { if (parts.length < CITE_AMOUNT_PARTS_WITH_VERSION) { return undefined; } const match = CITATION_VERSION_FIELD_REGEX.exec(parts[parts.length - 1]); if (!match) { return undefined; } return { kind: match[1] === 't' ? 'video' : 'attachment', version: match[2] }; -} +};As per path instructions,
src/main/webapp/**/*.tsrequirescode_style:arrow_funcs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/webapp/app/iris/overview/citation-text/iris-citation-text.util.ts` around lines 238 - 246, Convert the parseTrailingVersion helper from a function declaration to an arrow-function assignment, preserving its existing parameter type, return type, validation, regex matching, and result behavior.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/main/webapp/app/iris/overview/citation-text/iris-citation-text.util.ts`:
- Around line 238-246: Convert the parseTrailingVersion helper from a function
declaration to an arrow-function assignment, preserving its existing parameter
type, return type, validation, regex matching, and result behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 910fec2b-af6b-4529-b15d-933df0508743
⛔ Files ignored due to path filters (2)
src/main/resources/config/liquibase/changelog/20260729121844_changelog.xmlis excluded by!**/*.xmlsrc/main/resources/config/liquibase/master.xmlis excluded by!**/*.xml
📒 Files selected for processing (25)
src/main/java/de/tum/cit/aet/artemis/iris/service/IrisCitationService.javasrc/main/java/de/tum/cit/aet/artemis/iris/service/session/AbstractIrisChatSessionService.javasrc/main/java/de/tum/cit/aet/artemis/lecture/api/LectureUnitRepositoryApi.javasrc/main/java/de/tum/cit/aet/artemis/lecture/domain/LectureUnitProcessingState.javasrc/main/java/de/tum/cit/aet/artemis/lecture/dto/LectureUnitIngestedVersionsDTO.javasrc/main/java/de/tum/cit/aet/artemis/lecture/dto/LectureUnitMaterialVersionsDTO.javasrc/main/java/de/tum/cit/aet/artemis/lecture/repository/LectureUnitRepository.javasrc/main/java/de/tum/cit/aet/artemis/lecture/service/LectureContentProcessingService.javasrc/main/java/de/tum/cit/aet/artemis/lecture/service/ProcessingStateCallbackService.javasrc/main/java/de/tum/cit/aet/artemis/lecture/web/LectureUnitResource.javasrc/main/webapp/app/iris/overview/citation-text/iris-citation-material-version.service.tssrc/main/webapp/app/iris/overview/citation-text/iris-citation-text.component.spec.tssrc/main/webapp/app/iris/overview/citation-text/iris-citation-text.component.tssrc/main/webapp/app/iris/overview/citation-text/iris-citation-text.model.tssrc/main/webapp/app/iris/overview/citation-text/iris-citation-text.util.tssrc/main/webapp/app/lecture/overview/course-lectures/details/course-lecture-details.component.spec.tssrc/main/webapp/app/lecture/overview/course-lectures/details/course-lecture-details.component.tssrc/main/webapp/i18n/de/iris.jsonsrc/main/webapp/i18n/de/lectureUnit.jsonsrc/main/webapp/i18n/en/iris.jsonsrc/main/webapp/i18n/en/lectureUnit.jsonsrc/test/java/de/tum/cit/aet/artemis/iris/service/IrisCitationServiceTest.javasrc/test/java/de/tum/cit/aet/artemis/lecture/LectureUnitIntegrationTest.javasrc/test/java/de/tum/cit/aet/artemis/lecture/service/LectureContentProcessingServiceTest.javasrc/test/java/de/tum/cit/aet/artemis/lecture/service/ProcessingStateCallbackServiceClassificationTest.java
🚧 Files skipped from review as they are similar to previous changes (24)
- src/main/webapp/i18n/en/lectureUnit.json
- src/main/webapp/i18n/en/iris.json
- src/test/java/de/tum/cit/aet/artemis/lecture/service/LectureContentProcessingServiceTest.java
- src/main/java/de/tum/cit/aet/artemis/iris/service/session/AbstractIrisChatSessionService.java
- src/main/java/de/tum/cit/aet/artemis/lecture/api/LectureUnitRepositoryApi.java
- src/main/webapp/i18n/de/lectureUnit.json
- src/main/java/de/tum/cit/aet/artemis/lecture/dto/LectureUnitIngestedVersionsDTO.java
- src/main/java/de/tum/cit/aet/artemis/lecture/service/LectureContentProcessingService.java
- src/main/webapp/app/lecture/overview/course-lectures/details/course-lecture-details.component.spec.ts
- src/main/java/de/tum/cit/aet/artemis/lecture/dto/LectureUnitMaterialVersionsDTO.java
- src/main/webapp/i18n/de/iris.json
- src/main/webapp/app/iris/overview/citation-text/iris-citation-text.component.spec.ts
- src/main/java/de/tum/cit/aet/artemis/lecture/domain/LectureUnitProcessingState.java
- src/main/java/de/tum/cit/aet/artemis/lecture/web/LectureUnitResource.java
- src/main/webapp/app/lecture/overview/course-lectures/details/course-lecture-details.component.ts
- src/main/java/de/tum/cit/aet/artemis/lecture/repository/LectureUnitRepository.java
- src/main/webapp/app/iris/overview/citation-text/iris-citation-material-version.service.ts
- src/main/java/de/tum/cit/aet/artemis/lecture/service/ProcessingStateCallbackService.java
- src/test/java/de/tum/cit/aet/artemis/iris/service/IrisCitationServiceTest.java
- src/test/java/de/tum/cit/aet/artemis/lecture/service/ProcessingStateCallbackServiceClassificationTest.java
- src/main/webapp/app/iris/overview/citation-text/iris-citation-text.component.ts
- src/test/java/de/tum/cit/aet/artemis/lecture/LectureUnitIntegrationTest.java
- src/main/webapp/app/iris/overview/citation-text/iris-citation-text.model.ts
- src/main/java/de/tum/cit/aet/artemis/iris/service/IrisCitationService.java
ee7fa1e
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@vivienxr The current head correctly preserves citation versioning and unit-target navigation after removing the redundant deepLink parameter. The lifecycle, parsing, authorization, migration, and regression coverage remain consistent with the intended behavior. No required checks were configured in the captured CI snapshot, and no completed relevant check failed.
…f-slides-and-videos
|
There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions. |
Summary
Iris citations link to a specific slide page or video timestamp, but kept pointing at the same page or timestamp after slide re-upload or vidoes re-transcription — so a citation would look correct while silently landing on unrelated content (e.g. changed pages). This PR pins every citation to the version of the material it was generated from, re-checks it against the material's current state on click, and opens the lecture unit accordingly instead of jumping to a page or timestamp that possibly no longer means anything.
Checklist
General
Server
Client
authoritiesto all new routes and checked the course groups for displaying navigation elements (links, buttons).Motivation and Context
A citation's coordinates (e.g. page 7, timestamp 2:14) are only meaningful relative to one particular revision of the material. Chat messages with citations are kept forever, while lecture material is mutable: a lecturer inserting a single slide shifts every page number after it, and a re-transcribed video shifts every timestamp. Until now nothing recorded which revision an answer was written against, so there was no way to tell a citation that is still accurate from one that has quietly gone stale.
The failure mode is worse than a dead link. The citation still resolves, the student lands on a real page or a real timestamp, and nothing signals that this is no longer the passage Iris was talking about — which might costs trust. Slides could already be told apart via
Attachment#version, but the video side had no equivalent: a cited timestamp comes from a transcription segment, and the existingvideo_source_hashonly notices a changed URL, so re-transcribing a different video from the same link invalidates every timestamp without leaving any trace.Description
Videos now have a version number. Slides already had one (
Attachment#version), videos had nothing. This PR adds two columns tolecture_unit_processing_state(Liquibase20260729121844):transcription_versionandtranscription_content_hash. The version counts up whenever a unit's transcription changes. The hash is how we detect that it actually changed. Both are needed:LectureTranscription, because that row is deleted and recreated whenever the video changes. A counter stored there would restart at 1 every time, so a citation pinned to version 1 would look current forever.This is the same slide attachments already use: the hash detects the change, the version is the number that gets pinned.
Every citation records the version it was written against. Just before an assistant message is saved
IrisCitationService.stampCitationVersionsappends two fields to each citation:Only one of the two slots is ever filled. That is also how the client later knows whether a citation is about slides or a video, so it never has to work that out for itself.
A citation is only stamped once its unit finished processing (
ProcessingPhase.DONE), read viafindIngestedVersionsByIds. While a unit is being reprocessed, Iris still answers from the previous revision, so the current version numbers would describe the wrong material. If a citation cannot be stamped — unknown unit, or material with no version yet — it is left alone and behaves exactly as before.The check happens when the citation is clicked. A new endpoint
GET lecture-units/{lectureUnitId}/material-versions(restricted to students of the unit) returns the versions the unit has right now. The client calls it on click and compares:citation.outdated.staleAsking on click instead of when the chat loads means the answer reflects the material at that moment. Citations written before this PR carry no version fields, navigate straight through, and cause no request.
Existing citations and videos keep working unchanged. Nothing is rewritten and no backfill runs. Stamping happens on the write path only, so every message already in the database stays exactly as it is.
transcription_version = NULL. New video citations for them are left unstamped rather than pinned to a guessed number, so they behave like old citations until the unit is processed again, which sets the version to 1. Both new columns are nullable and added without a default, so existing rows are untouched.Summary:3:. Cosmetic only, but worth knowing.A cited unit or material that is gone is reported by the page that knows it. Every citation navigation carries
deepLink=true— stamped ones and unstamped ones alike, since the version check is skipped for the latter but the navigation is not. When the lecture page cannot find the requested unit among its own, it says so (lectureUnit.deepLink.unitGone) instead of silently dropping the target as it did before. So a citation written before this PR, which makes no request and pins nothing, still reports a unit that no longer exists; and a stamped citation whose unit is gone stays quiet on click (the version endpoint answers 403/404 there) and lets the lecture page do the talking, so the same situation is never worded twice in two different ways. It is deliberately phrased for the deep-link mechanism rather than for Iris, since a citation is only one way to arrive with a unit id.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
Performance Review
Code Review
Manual Tests
Test Coverage
Note: Some tests in the Test job did not pass (
failure). Coverage below may be partial.Client
Server
Last updated: 2026-08-17 13:55:00 UTC
Screenshots
Summary by CodeRabbit
New Features
Bug Fixes
Localization