Hyperion: Migrate FAQ rewrite functionality from Iris to Hyperion
#12043
+1,642
−757
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
tests
server
Contributor
WalkthroughReplaces Iris-based FAQ rewriting with a new Hyperion-based system. Introduces Java backend services (HyperionFaqRewriteService, HyperionFaqResource), TypeScript API clients, and prompt templates for FAQ rewriting and consistency checking. Removes Iris rewriting endpoints, services, and DTOs while migrating frontend components to use the new Hyperion API. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant HyperionFaqResource
participant HyperionFaqRewriteService
participant ChatClient
participant FaqRepository
participant TemplateService
Client->>HyperionFaqResource: POST /api/hyperion/courses/{courseId}/faq/rewrite<br/>RewriteFaqRequest
HyperionFaqResource->>HyperionFaqRewriteService: rewriteFaq(courseId, faqText)
rect rgba(100, 150, 200, 0.5)
Note over HyperionFaqRewriteService: Rewrite Phase
HyperionFaqRewriteService->>TemplateService: Render rewrite prompts<br/>(system + user)
TemplateService-->>HyperionFaqRewriteService: Prompts
HyperionFaqRewriteService->>ChatClient: Send rewrite request
ChatClient-->>HyperionFaqRewriteService: Rewritten text
end
rect rgba(150, 200, 150, 0.5)
Note over HyperionFaqRewriteService: Consistency Check Phase
HyperionFaqRewriteService->>FaqRepository: Fetch recent FAQs (max 10)
FaqRepository-->>HyperionFaqRewriteService: FAQ list
HyperionFaqRewriteService->>TemplateService: Render consistency check prompts<br/>(with existing FAQs + proposed text)
TemplateService-->>HyperionFaqRewriteService: Prompts
HyperionFaqRewriteService->>ChatClient: Send consistency check request
ChatClient-->>HyperionFaqRewriteService: JSON response<br/>(inconsistencies, suggestions, improvement)
end
HyperionFaqRewriteService-->>HyperionFaqResource: RewriteFaqResponseDTO
HyperionFaqResource-->>Client: 200 OK<br/>RewriteFaqResponse
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@src/main/java/de/tum/cit/aet/artemis/hyperion/web/HyperionFaqResource.java`:
- Around line 44-49: The controller method rewriteFaq is missing `@Valid` on the
`@RequestBody` parameter and the DTO lacks `@NotBlank`, so add
javax.validation.Valid to the request parameter in
HyperionFaqResource.rewriteFaq(`@PathVariable` long courseId, `@Valid` `@RequestBody`
RewriteFaqRequestDTO request) and annotate the faqText field (or corresponding
getter) in RewriteFaqRequestDTO with javax.validation.constraints.NotBlank
(instead of or in addition to `@NotNull`) to enforce non-empty input at the
controller boundary, mirroring the validation used by
ProblemStatementRewriteRequestDTO; ensure necessary imports are added so Spring
validation triggers before calling HyperionFaqRewriteService.
In `@src/main/resources/prompts/hyperion/rewrite_faq_system.st`:
- Around line 7-13: Fix the typos and stray backslash in the prompt rules:
remove the literal backslash after "bullet points," rewrite rule 8 to a single
smooth sentence ("If someone inputs a very short text that does not resemble an
answer to a potential question, respond accordingly and point out that the input
is too short."), fix "please make. sure" to "please respond accordingly", and
generally smooth wording for rules 4–8 so they read as complete, grammatical
instructions while preserving the intended meaning and Markdown formatting.
In `@src/main/resources/prompts/hyperion/rewrite_faq_user.st`:
- Around line 1-5: The template placeholder is wrong: the rendering code
(HyperionFaqRewriteService.renderTemplate / HyperionFaqRewriteService.java
around the template usage) provides the FAQ content under the context key
faqText, but the template uses {{rewritten_text}}, so the model gets empty
input; update the template in rewrite_faq_user.st to use {{faqText}} (or match
whatever key is used by HyperionFaqRewriteService) so the FAQ input is passed
into the prompt.
In `@src/main/webapp/app/openapi/.openapi-generator/VERSION`:
- Line 1: The VERSION file under src/main/webapp/app/openapi/.openapi-generator
currently contains "7.19.0" but gradle/openapi.gradle pins the
openapi-generator-gradle-plugin to "7.14.0", causing mismatch; either update the
plugin version in gradle/openapi.gradle (openapi-generator-gradle-plugin) to
7.19.0 to match the VERSION file, or regenerate the OpenAPI client code using
the 7.14.0 generator and update
src/main/webapp/app/openapi/.openapi-generator/VERSION to 7.14.0 so both the
VERSION file and the plugin (openapi-generator-gradle-plugin) are aligned.
In `@src/main/webapp/app/openapi/query.params.ts`:
- Around line 80-120: The toString method is collapsing exploded arrays into a
single "key=a,b" instead of emitting repeated key=value pairs; update toString
to iterate the record produced by toRecord() and for each entry: if the value is
an array (from toRecord's exploded branch) push one `${key}=${value}` per array
element, otherwise push a single `${key}=${value}`; use the encoded keys/values
already produced by toRecord() so that QueryParams.toString() and
QueryParams.toRecord() remain consistent (refer to the toString, toRecord
methods and encoder.encodeKey/encodeValue usage).
- Around line 134-159: The deep-object serialization currently calls
convertToString on null/undefined which throws; fix by guarding against
null/undefined before converting and by making convertToString defensive: in
concatHttpParamsObject (function concatHttpParamsObject) skip any keys/values
that are null or undefined (for arrays, filter out null/undefined before
mapping) and only call convertToString for non-null values, and update
convertToString to handle null/undefined (e.g., return an empty string) as a
safety net.
In
`@src/main/webapp/app/shared/monaco-editor/model/actions/artemis-intelligence/artemis-intelligence.service.spec.ts`:
- Around line 146-151: The test uses the outdated type FaqRewriteResponse for
mockResponse; update the type annotation to the corrected import
RewriteFaqResponse and ensure any other occurrences in
artemis-intelligence.service.spec.ts (e.g., the mockResponse declaration and any
typed variables or function parameters referencing FaqRewriteResponse) are
changed to RewriteFaqResponse so the test compiles against the fixed import.
- Around line 362-365: The test is still using the old type name
FaqRewriteResponse; update that type reference to the current response type used
by the rewriteFAQ API (match the type used elsewhere in your codebase/tests),
adjust the mockResponse object shape if needed to satisfy the new type, and
ensure mockHyperionApiService.rewriteFAQ.mockReturnValue(of(mockResponse)) uses
the updated typed mock. Target the declaration of mockResponse and the
FaqRewriteResponse identifier in this spec and replace it with the correct
exported response type name.
- Line 27: The import and type name are wrong: replace the import
"FaqRewriteResponse" from 'app/openapi/model/faqRewriteResponse' with the
correct import "RewriteFaqResponse" from 'app/openapi/model/rewriteFaqResponse',
and update all type usages of FaqRewriteResponse in this test (e.g., the
variables and assertions currently using FaqRewriteResponse) to use
RewriteFaqResponse instead so the file compiles.
🧹 Nitpick comments (10)
src/main/resources/prompts/hyperion/faq_consistency_system.st (1)
1-14: Specify an explicit output schema to stabilize parsing.
“Structured data requested” is vague; consider stating exact JSON keys to reduce model variance and downstream parsing errors.💡 Suggested addition
@@ -Provide ONLY the structured data requested. Do not add conversational filler. +Provide ONLY the structured data requested. Do not add conversational filler. + +Output JSON with keys: +{ + "inconsistent": boolean, + "conflictingFaqIds": string[], + "suggestions": string[], + "improvedAnswer": string +}src/main/java/de/tum/cit/aet/artemis/hyperion/service/HyperionPromptTemplateService.java (1)
63-75:renderObjecthas null-safety inconsistency and duplicatesrenderlogic.The
rendermethod (lines 29-52) now handles null values safely, butrenderObjectdoes not—callingentry.getValue().toString()on line 68 will throw an NPE if any value is null. Additionally, sinceMap<String, Object>is assignable toMap<String, ?>,renderObjectis now redundant.♻️ Consolidate by removing renderObject
Consider deprecating or removing
renderObjectentirely, asrendernow acceptsMap<String, ?>and handles null values safely. IfrenderObjectmust remain, apply the same null-safety:public String renderObject(String resourcePath, Map<String, Object> variables) { try { var resource = new ClassPathResource(resourcePath); String rendered = StreamUtils.copyToString(resource.getInputStream(), StandardCharsets.UTF_8); + + if (variables == null) { + return rendered; + } + for (var entry : variables.entrySet()) { - rendered = rendered.replace("{{" + entry.getKey() + "}}", entry.getValue().toString()); + String key = entry.getKey(); + Object value = entry.getValue(); + String stringValue = (value == null) ? "" : String.valueOf(value); + rendered = rendered.replace("{{" + key + "}}", stringValue); } return rendered; }src/main/java/de/tum/cit/aet/artemis/hyperion/dto/RewriteFaqRequestDTO.java (1)
11-12: Consider using@NotBlankinstead of@NotNullfor stronger validation.
@NotNullallows empty or whitespace-only strings, which are likely invalid for FAQ text input. Using@NotBlankensures the text is non-null, non-empty, and contains at least one non-whitespace character.♻️ Proposed fix
-import jakarta.validation.constraints.NotNull; +import jakarta.validation.constraints.NotBlank; `@JsonInclude`(JsonInclude.Include.NON_EMPTY) `@Schema`(description = "Request to rewrite a FAQ") -public record RewriteFaqRequestDTO(`@NotNull` String faqText) { +public record RewriteFaqRequestDTO(`@NotBlank` String faqText) { }Based on learnings, always validate and sanitize inputs in server-side Java code.
src/test/java/de/tum/cit/aet/artemis/hyperion/service/HyperionConsistencyCheckServiceTest.java (1)
34-34: Align test class name with filename for discoverability.The class name changed but the filename still reflects “Check”. It’s legal, yet it breaks naming conventions and makes search/discovery harder. Consider renaming the file or reverting the class name to match.
src/main/java/de/tum/cit/aet/artemis/hyperion/service/HyperionFaqRewriteService.java (5)
47-60: Javadoc is misplaced and incomplete.The Javadoc at lines 47-52 appears between the field declaration (
observationRegistry) and the constructor, making it confusing. Additionally, it only documents 2 parameters (chatClient,templateService) but the constructor takes 4 parameters.♻️ Proposed fix
- /** - * Creates a new HyperionFaqRewriteService. - * - * `@param` chatClient the AI chat client (optional) - * `@param` templateService prompt template service - */ private final ObservationRegistry observationRegistry; + /** + * Creates a new HyperionFaqRewriteService. + * + * `@param` faqRepository the repository for FAQ entities + * `@param` chatClient the AI chat client + * `@param` templates prompt template service + * `@param` observationRegistry the observation registry for metrics + */ public HyperionFaqRewriteService(FaqRepository faqRepository, ChatClient chatClient, HyperionPromptTemplateService templates, ObservationRegistry observationRegistry) {
62-68: Fix typo in Javadoc.Line 63 has a typo: "Executes the give n FAQ" should be "Executes the given FAQ" (or more accurately, "Rewrites the given FAQ").
♻️ Proposed fix
/** - * Executes the give n FAQ for the provided course + * Rewrites the given FAQ text for the provided course. * * `@param` courseId the id of the course context * `@param` faqText the FAQ to be rewritten
76-79: Remove dead commented code and clarify variable naming.Line 79 contains dead commented code that should be removed. Also, the map key
"rewritten_text"on line 76 is misleading since it contains the input text to be rewritten, not the output.♻️ Proposed fix
- Map<String, String> input = Map.of("rewritten_text", faqText.trim()); + Map<String, String> input = Map.of("faq_text", faqText.trim()); String systemPrompt = templateService.render(PROMPT_REWRITE_SYSTEM, Map.of()); String userPrompt = templateService.render(PROMPT_REWRITE_USER, input); - // String prompt = templateService.render(PROMPT_REWRITE, input);Note: Ensure the corresponding prompt template uses
{{faq_text}}placeholder if you apply this change.
115-117: FAQ limit may silently omit consistency issues.The
limit(10)on line 116 silently restricts consistency checks to only the 10 most recent FAQs. If a course has more FAQs, potential inconsistencies with older FAQs will be missed without any indication to the user.Consider one of:
- Document this limitation in the API response or logs
- Make the limit configurable
- Log a warning when FAQs are truncated
List<ConsistencyIssue.Faq> faqData = faqs.stream().limit(10).map(faq -> new ConsistencyIssue.Faq(faq.getId(), faq.getQuestionTitle(), faq.getQuestionAnswer())).toList(); + if (faqs.size() > 10) { + log.debug("Consistency check limited to 10 FAQs out of {} for course {}", faqs.size(), text); + }
143-147: Null element handling in parseInconsistencies.The method handles a null
faqslist but does not filter out potential null elements within the list, which could cause a NullPointerException if the AI returns malformed data.♻️ Defensive null filtering
private List<String> parseInconsistencies(List<ConsistencyIssue.Faq> faqs) { if (faqs == null) return List.of(); - return faqs.stream().map(f -> String.format("FAQ ID: %s, Title: %s, Answer: %s", f.id(), f.title(), f.answer())).toList(); + return faqs.stream() + .filter(f -> f != null) + .map(f -> String.format("FAQ ID: %s, Title: %s, Answer: %s", f.id(), f.title(), f.answer())) + .toList(); }src/main/webapp/app/openapi/query.params.ts (1)
55-78: Avoid object spread when merging ParamOptions.
Prefer an explicit merge to align with the project’s style guidance. As per coding guidelines, avoid object spread for objects.♻️ Suggested change
- if (options) { - entry.options = this.resolveOptions({...entry.options, ...options}); - } + if (options) { + entry.options = this.resolveOptions({ + explode: options.explode ?? entry.options.explode, + delimiter: options.delimiter ?? entry.options.delimiter, + }); + }
src/main/java/de/tum/cit/aet/artemis/hyperion/web/HyperionFaqResource.java
Show resolved
Hide resolved
...shared/monaco-editor/model/actions/artemis-intelligence/artemis-intelligence.service.spec.ts
Outdated
Show resolved
Hide resolved
...shared/monaco-editor/model/actions/artemis-intelligence/artemis-intelligence.service.spec.ts
Outdated
Show resolved
Hide resolved
...shared/monaco-editor/model/actions/artemis-intelligence/artemis-intelligence.service.spec.ts
Outdated
Show resolved
Hide resolved
github-project-automation
bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
|
@laadvo Test coverage could not be fully measured because some tests failed. Please check the workflow logs for details. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/main/webapp/app/shared/monaco-editor/model/actions/artemis-intelligence/artemis-intelligence.service.spec.ts`:
- Around line 43-47: TestBed is missing a provider for HyperionFaqApiService and
the mock uses the wrong method name; create a mockHyperionFaqApiService object
(or rename mockHyperionApiService) with a jest.fn() called rewriteFaq
(camelCase) and add { provide: HyperionFaqApiService, useValue:
mockHyperionFaqApiService } to the TestBed providers alongside the existing
HyperionProblemStatementApiService provider so ArtemisIntelligenceService can
inject both dependencies.
🧹 Nitpick comments (1)
src/main/webapp/app/shared/monaco-editor/model/actions/artemis-intelligence/artemis-intelligence.service.spec.ts (1)
360-370: Consider testing the loading state transition.The test verifies
isLoading()isfalsebefore and after the call, but sinceof()emits synchronously, it doesn't actually observe the loading state beingtrueduring execution. If you want to verify the loading indicator appears during async operations, consider using aSubjector delayed observable to control emission timing.This is optional since the current test still validates the expected end state.
...shared/monaco-editor/model/actions/artemis-intelligence/artemis-intelligence.service.spec.ts
Outdated
Show resolved
Hide resolved
|
@laadvo Test coverage could not be fully measured because some tests failed. Please check the workflow logs for details. |
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||
laadvo
force-pushed
the
chore/hyperion/move-iris-rewrite-to-hyperion
branch
from
8b78221 to
33c3ad4
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@src/main/java/de/tum/cit/aet/artemis/hyperion/service/HyperionFaqRewriteService.java`:
- Around line 62-68: Update the Javadoc for the method in
HyperionFaqRewriteService (the comment starting "Executes the give n FAQ for the
provided course") to correct the typo "give n FAQ" to "given FAQ" so the summary
reads "Executes the given FAQ for the provided course"; ensure the rest of the
`@param` and `@return` tags remain unchanged.
In `@src/main/java/de/tum/cit/aet/artemis/hyperion/web/HyperionFaqResource.java`:
- Around line 39-52: Update the Javadoc on HyperionFaqResource.rewriteFaq to
correct the `@param` request description: replace the incorrect "the id of the
course the FAQ belongs to" with a brief description that the parameter is the
RewriteFaqRequestDTO containing the FAQ text to be rewritten (e.g., "the request
DTO containing the FAQ text to rewrite"). Ensure the method JavaDoc references
RewriteFaqRequestDTO and matches the actual method signature.
🧹 Nitpick comments (2)
src/main/java/de/tum/cit/aet/artemis/hyperion/service/HyperionFaqRewriteService.java (2)
47-60: Fix misplaced Javadoc and inconsistent parameter naming.The Javadoc comment (lines 47-52) is incorrectly placed between a field declaration and the constructor. Additionally, the constructor parameter
templatesdoesn't match the field nametemplateService.♻️ Proposed fix
- /** - * Creates a new HyperionFaqRewriteService. - * - * `@param` chatClient the AI chat client (optional) - * `@param` templateService prompt template service - */ private final ObservationRegistry observationRegistry; + /** + * Creates a new HyperionFaqRewriteService. + * + * `@param` faqRepository the FAQ repository + * `@param` chatClient the AI chat client + * `@param` templateService prompt template service + * `@param` observationRegistry the observation registry for telemetry + */ - public HyperionFaqRewriteService(FaqRepository faqRepository, ChatClient chatClient, HyperionPromptTemplateService templates, ObservationRegistry observationRegistry) { + public HyperionFaqRewriteService(FaqRepository faqRepository, ChatClient chatClient, HyperionPromptTemplateService templateService, ObservationRegistry observationRegistry) { this.faqRepository = faqRepository; this.chatClient = chatClient; - this.templateService = templates; + this.templateService = templateService; this.observationRegistry = observationRegistry; }
72-74: Consider renaming the map key for clarity.The key
"rewritten_text"is misleading since at this pointfaqTextis the original text being sent for rewriting, not the result. Consider using a more accurate key like"faq_text"or"original_text"to match the prompt template variable semantics.
src/main/java/de/tum/cit/aet/artemis/hyperion/service/HyperionFaqRewriteService.java
Show resolved
Hide resolved
src/main/java/de/tum/cit/aet/artemis/hyperion/web/HyperionFaqResource.java
Show resolved
Hide resolved
|
@laadvo Test coverage could not be fully measured because some tests failed. Please check the workflow logs for details. |
End-to-End (E2E) Test Results Summary
|
|||||||||||||||||||||||||||
…-to-hyperion' into chore/hyperion/move-iris-rewrite-to-hyperion
|
@laadvo Test coverage has been automatically updated in the PR description. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@src/main/java/de/tum/cit/aet/artemis/hyperion/service/HyperionFaqRewriteService.java`:
- Line 121: Fix the typo in the formatter comment inside
HyperionFaqRewriteService by replacing the incorrect marker `// `@formatter`:onó`
with the correct `// `@formatter`:on` so the IDE/source formatter recognizes the
directive; locate this comment in the HyperionFaqRewriteService class (around
the formatter block) and update the trailing character to a plain 'n' without
any extra unicode characters.
In
`@src/test/java/de/tum/cit/aet/artemis/hyperion/resource/HyperionFaqRewriteResourceIntegrationTest.java`:
- Around line 26-27: The test uses a Mockito mock that isn't injected into
Spring, so replace the `@Mock` annotation on the faqRewriteService field in
HyperionFaqRewriteResourceIntegrationTest with `@MockitoBean` so the mock is
registered in the Spring context and your when(...) stubbing on
faqRewriteService will be used when the controller endpoint is invoked; ensure
the field name faqRewriteService and class
HyperionFaqRewriteResourceIntegrationTest are updated accordingly to match the
other integration tests that use `@MockitoBean`.
🧹 Nitpick comments (4)
src/main/java/de/tum/cit/aet/artemis/hyperion/service/HyperionFaqRewriteService.java (2)
47-60: Misplaced Javadoc comment.The Javadoc comment (lines 47-52) appears between field declarations rather than directly above the constructor. Move it immediately before the constructor.
📝 Proposed fix
private final HyperionPromptTemplateService templateService; - /** - * Creates a new HyperionFaqRewriteService. - * - * `@param` chatClient the AI chat client (optional) - * `@param` templateService prompt template service - */ private final ObservationRegistry observationRegistry; + /** + * Creates a new HyperionFaqRewriteService. + * + * `@param` faqRepository repository for FAQ entities + * `@param` chatClient the AI chat client + * `@param` templates prompt template service + * `@param` observationRegistry observation registry for metrics + */ public HyperionFaqRewriteService(FaqRepository faqRepository, ChatClient chatClient, HyperionPromptTemplateService templates, ObservationRegistry observationRegistry) {
100-132: Consider adding observability to the consistency check.The
rewriteFaqmethod includes observation tracking, but thecheckFaqConsistencymethod does not. For complete observability of the FAQ rewriting flow, consider adding observation events or a nested scope for the consistency check phase.src/test/java/de/tum/cit/aet/artemis/hyperion/service/HyperionFaqRewriteServiceTest.java (1)
37-51: Avoid static field for test data that is reset per test.
existingFaqis declared asstaticbut initialized in@BeforeEach. This works but is misleading—if tests run in parallel or the field were accessed before setup, it could cause issues. Use an instance field instead.📝 Proposed fix
private HyperionFaqRewriteService hyperionFaqRewriteService; - private static Faq existingFaq; + private Faq existingFaq; `@BeforeEach` void setup() {src/test/java/de/tum/cit/aet/artemis/hyperion/resource/HyperionFaqRewriteResourceIntegrationTest.java (1)
41-50: Test assertion incomplete — response body not validated.
postWithResponseBodyreturns the deserialized response, but the test does not assert on the returned value. Consider validating that the response matchesfaqResponse.📝 Proposed fix
`@Test` `@WithMockUser`(username = TEST_PREFIX + "tutor1", roles = "TA") void testRewriteFaq_Success() throws Exception { RewriteFaqRequestDTO faqRequest = new RewriteFaqRequestDTO("Old FAQ Text"); RewriteFaqResponseDTO faqResponse = new RewriteFaqResponseDTO("New Text", List.of(), List.of(), "Improved"); when(faqRewriteService.rewriteFaq(anyLong(), anyString())).thenReturn(faqResponse); - request.postWithResponseBody("/api/hyperion/courses/" + courseId + "/faq/rewrite", faqRequest, RewriteFaqResponseDTO.class, HttpStatus.OK); + RewriteFaqResponseDTO result = request.postWithResponseBody("/api/hyperion/courses/" + courseId + "/faq/rewrite", faqRequest, RewriteFaqResponseDTO.class, HttpStatus.OK); + assertThat(result).isNotNull(); + assertThat(result.rewrittenText()).isEqualTo("New Text"); }
src/main/java/de/tum/cit/aet/artemis/hyperion/service/HyperionFaqRewriteService.java
Outdated
Show resolved
Hide resolved
...java/de/tum/cit/aet/artemis/hyperion/resource/HyperionFaqRewriteResourceIntegrationTest.java
Outdated
Show resolved
Hide resolved
|
@laadvo Test coverage has been automatically updated in the PR description. |
End-to-End (E2E) Test Results Summary
|
|||||||||||||||||||||||||||||||||||||||
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
`@src/main/java/de/tum/cit/aet/artemis/hyperion/service/HyperionFaqRewriteService.java`:
- Around line 71-75: The call faqText.trim() in rewriteFaq can NPE if faqText is
null; update the rewriteFaq method to null- and blank-guard the input before
trimming (e.g., compute a sanitized string like sanitizedFaq = (faqText == null
? "" : faqText.trim()) and then check for sanitizedFaq.isBlank() and handle it
(return a meaningful error/empty response or throw an IllegalArgumentException)
instead of calling trim() directly when building Map.of("rewritten_text", ...);
modify the Map.of usage and any downstream logic in
rewriteFaq/RewriteFaqResponseDTO to use sanitizedFaq.
In
`@src/test/java/de/tum/cit/aet/artemis/hyperion/resource/HyperionFaqRewriteResourceIntegrationTest.java`:
- Around line 47-48: Remove the redundant POST invocation that causes duplicate
side effects: keep only the single call that assigns the response to
RewriteFaqResponseDTO result (the call to request.postWithResponseBody(...,
faqRequest, RewriteFaqResponseDTO.class, HttpStatus.OK)) and delete the earlier
identical call; ensure only the call that assigns to result remains and that
variables faqRequest and courseId are still used as before.
🧹 Nitpick comments (1)
src/main/java/de/tum/cit/aet/artemis/hyperion/service/HyperionFaqRewriteService.java (1)
102-109: Prefer limiting FAQs at the database layer instead of in-memory.
findAllByCourseIdOrderByCreatedDateDescloads all FAQs, thenlimit(10)trims in memory. For large courses this is wasted work. Add a repository method that limits in SQL (e.g.,findTop10ByCourseIdOrderByCreatedDateDesc) and use it here.♻️ Suggested refactor
- List<Faq> faqs = faqRepository.findAllByCourseIdOrderByCreatedDateDesc(courseId); + List<Faq> faqs = faqRepository.findTop10ByCourseIdOrderByCreatedDateDesc(courseId); @@ - List<ConsistencyIssue.Faq> faqData = faqs.stream().limit(10).map(faq -> new ConsistencyIssue.Faq(faq.getId(), faq.getQuestionTitle(), faq.getQuestionAnswer())).toList(); + List<ConsistencyIssue.Faq> faqData = faqs.stream().map(faq -> new ConsistencyIssue.Faq(faq.getId(), faq.getQuestionTitle(), faq.getQuestionAnswer())).toList();
src/main/java/de/tum/cit/aet/artemis/hyperion/service/HyperionFaqRewriteService.java
Show resolved
Hide resolved
...java/de/tum/cit/aet/artemis/hyperion/resource/HyperionFaqRewriteResourceIntegrationTest.java
Outdated
Show resolved
Hide resolved
|
@laadvo Test coverage could not be fully measured because some tests failed. Please check the workflow logs for details. |
helios-aet
bot
temporarily deployed
to
artemis-test6.artemis.cit.tum.de
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||||||||||||||
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/main/java/de/tum/cit/aet/artemis/hyperion/service/HyperionFaqRewriteService.java`:
- Around line 114-119: The prompt is receiving faqData as a Java List which will
call toString() instead of JSON; before calling templateService.renderObject for
PROMPT_CONSISTENCY_USER, serialize faqData to JSON (e.g., use an ObjectMapper to
call writeValueAsString(faqData)) and pass that JSON string in the Map under
"faqs" (keeping other keys like "final_result" and "format" unchanged) so the
template gets valid JSON; update the code around faqData,
templateService.renderObject(...) and ensure any checked exceptions from
serialization are handled or propagated appropriately.
src/main/java/de/tum/cit/aet/artemis/hyperion/service/HyperionFaqRewriteService.java
Outdated
Show resolved
Hide resolved
|
@laadvo Test coverage has been automatically updated in the PR description. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/main/java/de/tum/cit/aet/artemis/hyperion/service/HyperionFaqRewriteService.java`:
- Around line 106-109: The current null/blank check for rewrittenText in
HyperionFaqRewriteService returns an empty string in the RewriteFaqResponseDTO
which may discard the user's original FAQ; change the fallback to return the
normalized input FAQ instead of "" — i.e., when rewrittenText == null ||
rewrittenText.isBlank(), construct the RewriteFaqResponseDTO using the
previously computed normalized input FAQ (the normalized FAQ variable used
earlier in the method, e.g., normalizedFaqText or similar) so the original
content is preserved; keep the same warnings/logging (log.warn) and other
response fields unchanged.
🧹 Nitpick comments (1)
src/main/java/de/tum/cit/aet/artemis/hyperion/service/HyperionFaqRewriteService.java (1)
115-120: Optimize FAQ retrieval to fetch only the top 10 FAQs at the database level.The current implementation loads all FAQs from the database and limits them in memory (
limit(10)), which is inefficient for courses with many FAQs. Add a repository methodfindTop10ByCourseIdOrderByCreatedDateDescusing Spring Data JPA'sfindTop<N>Byconvention to fetch only 10 FAQs directly from the database.
src/main/java/de/tum/cit/aet/artemis/hyperion/service/HyperionFaqRewriteService.java
Show resolved
Hide resolved
|
@laadvo Test coverage has been automatically updated in the PR description. |
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
laadvo
commented
Jan 27, 2026
|
|
||
| jvmArgs = [ | ||
| "-Dspring.profiles.active=test,artemis,core,atlas,scheduling,athena,apollon,iris,aeolus,theia,dev", | ||
| "-Dspring.profiles.active=test,artemis,core,atlas,scheduling,athena,apollon,iris,aeolus,theia,dev,localci,localvc", |
Contributor
Author
There was a problem hiding this comment.
Added the two profiles to be able to run ./gradlew generateApiDocs -x webapp
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@src/main/java/de/tum/cit/aet/artemis/hyperion/service/HyperionFaqRewriteService.java`:
- Around line 119-125: Add a repository-level limit so the DB returns only the
latest 10 FAQs and update the service to stop loading everything into memory:
add a new method findTop10ByCourseIdOrderByCreatedDateDesc(...) to FaqRepository
(or implement equivalent pagination), then change
HyperionFaqRewriteService.checkFaqConsistency to call that new repository method
and remove the stream().limit(10) usage when building faqData (keep mapping to
ConsistencyIssue.Faq as before); additionally, scan other usages of
findAllByCourseIdOrderByCreatedDateDesc (e.g., FaqImportService and FaqResource)
and replace with paged/top10 queries where appropriate.
src/main/java/de/tum/cit/aet/artemis/hyperion/service/HyperionFaqRewriteService.java
Show resolved
Hide resolved
|
@laadvo Test coverage has been automatically updated in the PR description. |
End-to-End (E2E) Test Results Summary
|
||||||||||||||||||
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR migrates the FAQ rewriting functionality from the Iris module to the Hyperion module to transition to SpringAI.
Checklist
General
Server
Client
Motivation and Context
This PR migrates the FAQ rewriting functionality from the Iris module to the Hyperion module. This move aligns the FAQ feature with existing Hyperion services, such as Problem Statement Generation and Problem Statement Rewrite, which served as the architectural inspiration for this migration.
We want to slowly move from LangChain to SpringAI. By integrating Spring AI directly, we remove the dependency on external middleware services.
Description
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
Client
Server
Last updated: 2026-01-27 13:54:52 UTC
Screenshots
FAQ before rewrite
Artemis Intelligence > Rewrite
FAQ after rewrite
FAQ inconsistencies textbox
Summary by CodeRabbit
New Features
Removed
Updates
✏️ Tip: You can customize this high-level summary in your review settings.