Skip to content

Conversation

@toukhi
Copy link
Contributor

@toukhi toukhi commented Nov 19, 2025

Checklist

General

  • I tested all changes and their related features with all corresponding user types on a test server.
  • This is a small issue that I tested locally and was confirmed by another developer on a test server.
  • I chose a title conforming to the naming conventions for pull requests.

Server

Motivation and Context

The TextExerciseChatPipelineExecutionDTO was refactored for improved consistency in Iris on edutelligence (see PR). Due to this refactoring, the expected structure and argument names have changed.
This PR updates the codebase to pass the correct arguments in the updated format, ensuring compatibility with the new DTO structure.

Description

This PR updates the PyrisTextExerciseChatPipelineExecutionDTO to match the new structure introduced in edutelligence.
The DTO was changed from:

public record PyrisTextExerciseChatPipelineExecutionDTO(PyrisPipelineExecutionDTO execution, PyrisTextExerciseDTO exercise, String sessionTitle, List<PyrisMessageDTO> conversation, String currentSubmission, @Nullable String customInstructions)

to the updated format:

public record PyrisTextExerciseChatPipelineExecutionDTO(PyrisTextExerciseDTO exercise, String sessionTitle, List<PyrisMessageDTO> chatHistory, PyrisUserDTO user, String currentSubmission, PyrisPipelineExecutionSettingsDTO settings, List<PyrisStageDTO> initialStages, @Nullable String customInstructions)

All necessary adjustments were made throughout the codebase to ensure that the new arguments are passed correctly and that all usages remain consistent with the updated DTO.

Steps for Testing

Warning

This PR can only be tested together with the corresponding PR on edutelligence.
To test this PR on a test server, you need to add the label 'deploy:pyris-test' in the corresponding PR on edutelligence.

  1. Log in to Artemis
  2. Enable Artemis
  3. Navigate to a text exercise of a course (if there is none, just create one)
  4. Write something and click on 'Submit'
  5. After submitting, click on the Iris icon that appeared on the bottom right
  6. Verify that you can text with Iris and ask it questions
  7. If you are testing on a test server: Don't forget to remove the label 'deploy:pyris-test' in the corresponding PR on edutelligence when you are done with testing

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

  • I (as a reviewer) confirm that the client changes (in particular related to REST calls and UI responsiveness) are implemented with a very good performance even for very large courses with more than 2000 students.
  • I (as a reviewer) confirm that the server changes (in particular related to database calls) are implemented with a very good performance even for very large courses with more than 2000 students.

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Summary by CodeRabbit

  • Refactor
    • Restructured internal data organization for chat pipeline communication to improve code maintainability and data flow clarity across the system.

✏️ Tip: You can customize this high-level summary in your review settings.

@toukhi toukhi requested a review from a team as a code owner November 19, 2025 23:49
@github-project-automation github-project-automation bot moved this to Work In Progress in Artemis Development Nov 19, 2025
@github-actions github-actions bot added tests server Pull requests that update Java code. (Added Automatically!) iris Pull requests that affect the corresponding module labels Nov 19, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 19, 2025

Walkthrough

This pull request restructures the PyrisTextExerciseChatPipelineExecutionDTO record by flattening its design. The nested execution field is removed, and its contents (settings, initialStages) are promoted to top-level fields alongside a new user field. The conversation field is renamed to chatHistory. Callers are updated to use the new DTO structure.

Changes

Cohort / File(s) Summary
DTO Structural Redesign
src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/chat/textexercise/PyrisTextExerciseChatPipelineExecutionDTO.java
Record signature refactored: removed PyrisPipelineExecutionDTO execution, added PyrisUserDTO user, PyrisPipelineExecutionSettingsDTO settings, List<PyrisStageDTO> initialStages; renamed conversation to chatHistory. Imports updated to reflect new dependencies.
DTO Constructor Usage
src/main/java/de/tum/cit/aet/artemis/iris/service/session/IrisTextExerciseChatSessionService.java
Updated PyrisTextExerciseChatPipelineExecutionDTO construction to pass new DTO structure with flattened fields including PyrisUserDTO instance and direct access to settings() and initialStages(). Added PyrisUserDTO import.
Test Assertions
src/test/java/de/tum/cit/aet/artemis/iris/IrisTextExerciseChatMessageIntegrationTest.java
Updated token access path from dto.execution().settings().authenticationToken() to dto.settings().authenticationToken() and initial stages from dto.execution().initialStages() to dto.initialStages() across test response handlers.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–25 minutes

  • The refactoring flattens a public DTO structure, creating a breaking change that affects constructor calls across multiple files
  • Field reordering and renaming require verification that all callers are updated consistently
  • Test files may need additional scrutiny to ensure access paths are correctly updated throughout the codebase
  • Consider checking whether other callers of PyrisTextExerciseChatPipelineExecutionDTO exist outside the shown files

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Iris: Refactor textExerciseChatPipelineExecutionDto' directly and clearly summarizes the main change—refactoring of the PyrisTextExerciseChatPipelineExecutionDTO DTO to match the new edutelligence structure.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch chore/iris/refactor-text-exercise-chat-pipeline-execution-dto

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (2)
src/test/java/de/tum/cit/aet/artemis/iris/IrisTextExerciseChatMessageIntegrationTest.java (1)

335-339: Session‑title update test remains valid with flattened DTO

Switching to dto.settings().authenticationToken() and dto.initialStages() keeps this test compatible with the new DTO while still validating the title update behavior. As a minor improvement, you could also assert on dto.sessionTitle() or dto.chatHistory() here if you want coverage of additional DTO fields, but it’s optional.

src/main/java/de/tum/cit/aet/artemis/iris/service/session/IrisTextExerciseChatSessionService.java (1)

34-35: Constructor wiring for new DTO shape looks correct

The added PyrisUserDTO import and the updated PyrisTextExerciseChatPipelineExecutionDTO construction (exercise, title, conversation/chatHistory, new PyrisUserDTO(user), latest submission text, dto.settings(), dto.initialStages(), settings.customInstructions()) align with the new record definition and keep null‑handling for custom instructions intact.

If you touch this again, consider renaming the local conversation variable to chatHistory to mirror the DTO field name.

Also applies to: 117-124

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fa8bb23 and 8cbba0a.

📒 Files selected for processing (3)
  • src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/chat/textexercise/PyrisTextExerciseChatPipelineExecutionDTO.java (1 hunks)
  • src/main/java/de/tum/cit/aet/artemis/iris/service/session/IrisTextExerciseChatSessionService.java (2 hunks)
  • src/test/java/de/tum/cit/aet/artemis/iris/IrisTextExerciseChatMessageIntegrationTest.java (5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/test/java/**/*.java

⚙️ CodeRabbit configuration file

test_naming: descriptive; test_size: small_specific; fixed_data: true; junit5_features: true; assert_use: assertThat; assert_specificity: true; archunit_use: enforce_package_rules; db_query_count_tests: track_performance; util_service_factory_pattern: true; avoid_db_access: true; mock_strategy: static_mocks; context_restart_minimize: true

Files:

  • src/test/java/de/tum/cit/aet/artemis/iris/IrisTextExerciseChatMessageIntegrationTest.java
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/iris/service/pyris/dto/chat/textexercise/PyrisTextExerciseChatPipelineExecutionDTO.java
  • src/main/java/de/tum/cit/aet/artemis/iris/service/session/IrisTextExerciseChatSessionService.java
🧠 Learnings (8)
📓 Common learnings
Learnt from: alexjoham
Repo: ls1intum/Artemis PR: 9455
File: src/test/java/de/tum/cit/aet/artemis/iris/IrisTextExerciseChatMessageIntegrationTest.java:401-401
Timestamp: 2024-10-15T11:33:17.915Z
Learning: In the Artemis project, when new fields are added to classes like `PyrisChatStatusUpdateDTO`, corresponding tests may be implemented in separate integration test classes such as `IrisChatTokenTrackingIntegrationTest`.
Learnt from: magaupp
Repo: ls1intum/Artemis PR: 8802
File: src/main/resources/templates/rust/exercise/src/context.rs:1-1
Timestamp: 2024-10-08T15:35:42.972Z
Learning: Code inside the `exercise` directories in the Artemis platform is provided to students and is intended for them to implement. TODO comments in these directories are meant to guide students and should not be addressed in the PR.
Learnt from: magaupp
Repo: ls1intum/Artemis PR: 8802
File: src/main/resources/templates/rust/exercise/src/context.rs:1-1
Timestamp: 2024-08-05T00:11:50.650Z
Learning: Code inside the `exercise` directories in the Artemis platform is provided to students and is intended for them to implement. TODO comments in these directories are meant to guide students and should not be addressed in the PR.
📚 Learning: 2024-10-15T11:33:17.915Z
Learnt from: alexjoham
Repo: ls1intum/Artemis PR: 9455
File: src/test/java/de/tum/cit/aet/artemis/iris/IrisTextExerciseChatMessageIntegrationTest.java:401-401
Timestamp: 2024-10-15T11:33:17.915Z
Learning: In the Artemis project, when new fields are added to classes like `PyrisChatStatusUpdateDTO`, corresponding tests may be implemented in separate integration test classes such as `IrisChatTokenTrackingIntegrationTest`.

Applied to files:

  • src/test/java/de/tum/cit/aet/artemis/iris/IrisTextExerciseChatMessageIntegrationTest.java
  • src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/chat/textexercise/PyrisTextExerciseChatPipelineExecutionDTO.java
  • src/main/java/de/tum/cit/aet/artemis/iris/service/session/IrisTextExerciseChatSessionService.java
📚 Learning: 2024-11-26T20:43:17.588Z
Learnt from: magaupp
Repo: ls1intum/Artemis PR: 9751
File: src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.java:143-148
Timestamp: 2024-11-26T20:43:17.588Z
Learning: In `src/test/java/de/tum/cit/aet/artemis/programming/util/ProgrammingExerciseFactory.java`, the test package name assigned in `populateUnreleasedProgrammingExercise` does not need to adhere to naming conventions.

Applied to files:

  • src/test/java/de/tum/cit/aet/artemis/iris/IrisTextExerciseChatMessageIntegrationTest.java
📚 Learning: 2025-09-25T20:28:36.905Z
Learnt from: SamuelRoettgermann
Repo: ls1intum/Artemis PR: 11419
File: src/main/java/de/tum/cit/aet/artemis/exam/domain/ExamUser.java:16-17
Timestamp: 2025-09-25T20:28:36.905Z
Learning: In the Artemis codebase, ExamUser entity uses ExamSeatDTO as a transient field for performance reasons. SamuelRoettgermann tested domain value objects but they caused 60x slower performance. This architectural exception is approved by maintainers due to significant performance benefits and Artemis naming convention requirements.

Applied to files:

  • src/test/java/de/tum/cit/aet/artemis/iris/IrisTextExerciseChatMessageIntegrationTest.java
  • src/main/java/de/tum/cit/aet/artemis/iris/service/session/IrisTextExerciseChatSessionService.java
📚 Learning: 2025-04-14T11:08:04.129Z
Learnt from: alexjoham
Repo: ls1intum/Artemis PR: 10666
File: src/main/webapp/app/iris/overview/services/iris-chat-http.service.ts:73-86
Timestamp: 2025-04-14T11:08:04.129Z
Learning: The IrisTutorSuggestionRequestMessage class in src/main/webapp/app/iris/shared/entities/iris-message.model.ts does not have a messageDifferentiator property, unlike IrisUserMessage. Therefore, messageDifferentiator should not be set in the createTutorSuggestion method in the IrisChatHttpService.

Applied to files:

  • src/test/java/de/tum/cit/aet/artemis/iris/IrisTextExerciseChatMessageIntegrationTest.java
📚 Learning: 2025-07-07T11:43:11.736Z
Learnt from: bassner
Repo: ls1intum/Artemis PR: 10705
File: src/test/java/de/tum/cit/aet/artemis/lecture/service/SlideUnhideServiceTest.java:54-57
Timestamp: 2025-07-07T11:43:11.736Z
Learning: In the Artemis test framework, the AbstractArtemisIntegrationTest base class provides common MockitoSpyBean fields like instanceMessageSendService as protected fields, making them available to all test subclasses through inheritance.

Applied to files:

  • src/test/java/de/tum/cit/aet/artemis/iris/IrisTextExerciseChatMessageIntegrationTest.java
📚 Learning: 2025-05-26T15:37:20.890Z
Learnt from: bassner
Repo: ls1intum/Artemis PR: 10782
File: src/main/java/de/tum/cit/aet/artemis/iris/service/session/IrisCourseChatSessionService.java:130-130
Timestamp: 2025-05-26T15:37:20.890Z
Learning: The Pyris service expects to receive null as customInstructions when there are no custom instructions configured. Null values should be passed directly to PyrisPipelineService.executeCourseChatPipeline without defensive null handling or conversion to empty strings.

Applied to files:

  • src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/chat/textexercise/PyrisTextExerciseChatPipelineExecutionDTO.java
  • src/main/java/de/tum/cit/aet/artemis/iris/service/session/IrisTextExerciseChatSessionService.java
📚 Learning: 2024-11-29T23:02:09.475Z
Learnt from: kaancayli
Repo: ls1intum/Artemis PR: 9740
File: src/main/java/de/tum/cit/aet/artemis/iris/service/session/IrisLectureChatSessionService.java:126-126
Timestamp: 2024-11-29T23:02:09.475Z
Learning: Token tracking should be implemented for all Iris session types, including `IrisLectureChatSessionService`, by extending `AbstractIrisChatSessionService` where appropriate.

Applied to files:

  • src/main/java/de/tum/cit/aet/artemis/iris/service/session/IrisTextExerciseChatSessionService.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-tests
  • GitHub Check: server-style
  • GitHub Check: client-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: bean-instantiation-check
  • GitHub Check: Analyse
🔇 Additional comments (5)
src/test/java/de/tum/cit/aet/artemis/iris/IrisTextExerciseChatMessageIntegrationTest.java (4)

104-107: Updated access to settings and stages matches new DTO layout

Using dto.settings().authenticationToken() and dto.initialStages() here correctly reflects the flattened PyrisTextExerciseChatPipelineExecutionDTO structure; behavior stays equivalent to the previous dto.execution().… access.


171-175: Consistent DTO field usage across both pipeline mocks

Both mocks in this test now read the token from dto.settings() and stages from dto.initialStages(), which is consistent with the new DTO signature and with the single‑message test above.

Also applies to: 179-185


271-275: Resend flow aligned with new DTO structure

The resend test’s assertions on dto.settings().authenticationToken() and dto.initialStages() are correctly adapted to the refactored DTO and keep the original semantics.


296-300: Rate‑limit test uses new settings/initialStages fields correctly

The rate‑limit scenario now also accesses the authentication token and stages via the top‑level settings and initialStages fields, which fits the new DTO design.

src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/chat/textexercise/PyrisTextExerciseChatPipelineExecutionDTO.java (1)

9-17: Flattened DTO design matches intended API evolution

The new PyrisTextExerciseChatPipelineExecutionDTO record cleanly exposes exercise, session title, chat history, user, submission text, settings, initial stages, and optional custom instructions, matching how callers now construct it. This aligns with the refactor away from the nested execution DTO.

@github-actions
Copy link

End-to-End (E2E) Test Results Summary

TestsPassed ☑️Skipped ⚠️Failed ❌️Time ⏱
End-to-End (E2E) Test Report205 ran200 passed3 skipped2 failed1h 12m 53s 532ms
TestResultTime ⏱
End-to-End (E2E) Test Report
e2e/exam/test-exam/TestExamParticipation.spec.ts
ts.Test exam participation › Early Hand-in › Using exercise overview to navigate within exam❌ failure3m 40s 40ms
e2e/exercise/programming/ProgrammingExerciseStaticCodeAnalysis.spec.ts
ts.Static code analysis tests › Configures SCA grading and makes a successful submission with SCA errors❌ failure2m 34s 328ms

Copy link
Contributor

@Anishyou Anishyou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image Tested locally works as expected

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

iris Pull requests that affect the corresponding module server Pull requests that update Java code. (Added Automatically!) tests

Projects

Status: Work In Progress

Development

Successfully merging this pull request may close these issues.

3 participants