Exercise variants: address the latest review round

A failed websocket publish could destroy a finished variant. Every terminal
transition writes the new phase first and publishes afterwards, so when the
publish threw, the exception escaped complete() into the pipeline's terminal
catch — which deleted the generated exercise and overwrote COMPLETED with
FAILED. That catch was introduced in the previous commit; before it existed,
the task service's already-terminal check kept the job intact.

Fix both ends: publish now swallows and logs a delivery failure, because
notifying the client is best effort and must never propagate into
state-changing code, and the terminal catch keeps the variant when the job has
already reached a terminal phase. Cover both with a test class that stubs the
websocket service to throw.

Give the wizard's free-form controls accessible names. The domain input, the
custom-instructions textarea and the six new-group settings had a styled <p> or
<div> beside them instead of a label, so a screen reader announced no name.
Each control now has an id and a matching <label for>; the label rules get
display:block since labels are inline. The two radio-group headings stay as
they are — a <label for> cannot address a group, and each radio already carries
its own label.

Name the AI variant action. The tooltip key read "Variante erstellen" /
"Create Variant", so neither locale mentioned AI; both now say so. The button is
icon-only and a tooltip is not an accessible name, so it also gets an aria-label
from the same key.

Abort the E2E runner when the mock LLM never becomes ready, instead of warning
and then pointing Spring AI at a dead port.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: Lara Dvorsek

run-e2e-tests-local-fast.sh

@@ -386,7 +386,14 @@ if [ "$SKIP_SERVER" = false ]; then
         if [ "$MOCK_READY" = true ]; then
             echo -e "${GREEN}Mock LLM is listening (PID $MOCK_LLM_PID).${NC}"
         else
-            echo -e "${RED}WARNING: mock LLM did not become ready; Hyperion tests may skip/fail.${NC}"
+            # Continuing would enable Hyperion and point Spring AI at a dead port, so the variant suite would
+            # fail later with connection errors that say nothing about the real cause.
+            echo -e "${RED}ERROR: mock LLM did not become ready; aborting instead of running Hyperion against a dead endpoint.${NC}"
+            if kill -0 "$MOCK_LLM_PID" 2>/dev/null; then
+                kill_tree "$MOCK_LLM_PID"
+            fi
+            rm -f "$LOCAL_DIR/hyperion-mock-llm.pid"
+            exit 1
         fi
         export ARTEMIS_HYPERION_ENABLED="true"
         export SPRING_AI_OPENAI_BASE_URL="http://localhost:${HYPERION_MOCK_LLM_PORT}/v1"

src/main/java/de/tum/cit/aet/artemis/hyperion/service/variants/ExerciseVariantGenerationPipelineService.java

@@ -215,6 +215,14 @@ public void run(VariantJob job) {
             // ExerciseVariantTaskService, which marks the job FAILED but never deletes the exercise — leaving the
             // clone, its repositories and its build plans behind as orphans. No failure summary here: that is an
             // extra LLM call on a path we already know is broken.
+            if (jobService.getJob(jobId, job.getInitiatorLogin()).map(current -> current.getPhase().isTerminal()).orElse(true)) {
+                // The job already reached a terminal phase, so the throw came from AFTER that transition
+                // (telemetry logging, the websocket publish). The variant is generated and the record says so —
+                // deleting it here would discard verified work over a failed notification.
+                log.error("Variant generation job {} raised an error after it had already finished (exercise {}); keeping the variant", jobId, job.getSourceExerciseId(),
+                        unexpected);
+                return;
+            }
             cleanupProvisionedVariant(variant, jobId);
             jobService.fail(jobId, "Unexpected error: " + unexpected.getMessage(), null);
             log.error("Variant generation job {} failed unexpectedly (exercise {})", jobId, job.getSourceExerciseId(), unexpected);

src/main/java/de/tum/cit/aet/artemis/hyperion/service/variants/ExerciseVariantJobService.java

@@ -492,7 +492,18 @@ private VariantJob mutate(String jobId, Consumer<VariantJob> mutation) {
         }
     }
 
+    /**
+     * Notifies the initiator about a job transition. Best effort by design: every caller publishes AFTER it has
+     * already written the new phase, so letting a delivery failure propagate would let a lost notification undo
+     * persisted state (the pipeline's terminal catch would treat it as a failed job). The client falls back to
+     * polling the job endpoints, so a dropped event costs freshness, not correctness.
+     */
     private void publish(VariantJob job, VariantGenerationEventDTO event) {
-        websocketService.send(job.getInitiatorLogin(), TOPIC_SUFFIX_PREFIX + job.getJobId(), event);
+        try {
+            websocketService.send(job.getInitiatorLogin(), TOPIC_SUFFIX_PREFIX + job.getJobId(), event);
+        }
+        catch (RuntimeException e) {
+            log.warn("Could not publish the {} event of variant job {}", event.type(), job.getJobId(), e);
+        }
     }
 }

src/main/webapp/app/course/manage/exercises/create-variant-modal/exercise-variant-ai-modal-wizard.component.html

@@ -170,8 +170,9 @@
             }
             @if (changeDomain()) {
                 <div class="field-group">
-                    <p class="field-group__label">{{ 'artemisApp.exerciseVariantGeneration.wizard.configure.domain' | artemisTranslate }}</p>
+                    <label class="field-group__label" for="wiz-domain">{{ 'artemisApp.exerciseVariantGeneration.wizard.configure.domain' | artemisTranslate }}</label>
                     <input
+                        id="wiz-domain"
                         tumUiInput
                         class="w-full"
                         [ngModel]="domainText()"
@@ -206,8 +207,9 @@
             }
             @if (changeCustom()) {
                 <div class="field-group">
-                    <p class="field-group__label">{{ 'artemisApp.exerciseVariantGeneration.wizard.configure.custom' | artemisTranslate }}</p>
+                    <label class="field-group__label" for="wiz-custom">{{ 'artemisApp.exerciseVariantGeneration.wizard.configure.custom' | artemisTranslate }}</label>
                     <textarea
+                        id="wiz-custom"
                         tumUiTextarea
                         rows="4"
                         class="w-full"
@@ -272,9 +274,12 @@
                                 <div class="new-group-settings" (click)="$event.stopPropagation()">
                                     <div class="flex flex-wrap gap-2 items-end mt-2">
                                         <div class="grow" style="min-width: 120px">
-                                            <div class="new-group-settings__label">{{ 'artemisApp.exerciseVariantGeneration.wizard.placement.groupTitle' | artemisTranslate }}</div>
+                                            <label class="new-group-settings__label" for="wiz-group-title">{{
+                                                'artemisApp.exerciseVariantGeneration.wizard.placement.groupTitle' | artemisTranslate
+                                            }}</label>
                                             <input
                                                 type="text"
+                                                id="wiz-group-title"
                                                 [ngModel]="newGroupTitle()"
                                                 (ngModelChange)="newGroupTitle.set($event)"
                                                 tumUiInput
@@ -283,9 +288,12 @@
                                             />
                                         </div>
                                         <div>
-                                            <div class="new-group-settings__label">{{ 'artemisApp.exerciseVariantGeneration.wizard.placement.maxPoints' | artemisTranslate }}</div>
+                                            <label class="new-group-settings__label" for="wiz-group-max-points">{{
+                                                'artemisApp.exerciseVariantGeneration.wizard.placement.maxPoints' | artemisTranslate
+                                            }}</label>
                                             <input
                                                 type="number"
+                                                id="wiz-group-max-points"
                                                 [ngModel]="newGroupMaxPoints()"
                                                 (ngModelChange)="newGroupMaxPoints.set($event !== '' ? +$event : undefined)"
                                                 tumUiInput
@@ -296,9 +304,12 @@
                                     </div>
                                     <div class="flex flex-wrap gap-2 mt-2">
                                         <div>
-                                            <div class="new-group-settings__label">{{ 'artemisApp.exerciseVariantGeneration.wizard.placement.release' | artemisTranslate }}</div>
+                                            <label class="new-group-settings__label" for="wiz-group-release">{{
+                                                'artemisApp.exerciseVariantGeneration.wizard.placement.release' | artemisTranslate
+                                            }}</label>
                                             <input
                                                 type="datetime-local"
+                                                id="wiz-group-release"
                                                 [ngModel]="newGroupReleaseDate()"
                                                 (ngModelChange)="newGroupReleaseDate.set($event)"
                                                 tumUiInput
@@ -307,9 +318,12 @@
                                             />
                                         </div>
                                         <div>
-                                            <div class="new-group-settings__label">{{ 'artemisApp.exerciseVariantGeneration.wizard.placement.start' | artemisTranslate }}</div>
+                                            <label class="new-group-settings__label" for="wiz-group-start">{{
+                                                'artemisApp.exerciseVariantGeneration.wizard.placement.start' | artemisTranslate
+                                            }}</label>
                                             <input
                                                 type="datetime-local"
+                                                id="wiz-group-start"
                                                 [ngModel]="newGroupStartDate()"
                                                 (ngModelChange)="newGroupStartDate.set($event)"
                                                 tumUiInput
@@ -318,9 +332,12 @@
                                             />
                                         </div>
                                         <div>
-                                            <div class="new-group-settings__label">{{ 'artemisApp.exerciseVariantGeneration.wizard.placement.due' | artemisTranslate }}</div>
+                                            <label class="new-group-settings__label" for="wiz-group-due">{{
+                                                'artemisApp.exerciseVariantGeneration.wizard.placement.due' | artemisTranslate
+                                            }}</label>
                                             <input
                                                 type="datetime-local"
+                                                id="wiz-group-due"
                                                 [ngModel]="newGroupDueDate()"
                                                 (ngModelChange)="newGroupDueDate.set($event)"
                                                 tumUiInput
@@ -329,11 +346,12 @@
                                             />
                                         </div>
                                         <div>
-                                            <div class="new-group-settings__label">
+                                            <label class="new-group-settings__label" for="wiz-group-assessment-due">
                                                 {{ 'artemisApp.exerciseVariantGeneration.wizard.placement.assessmentDue' | artemisTranslate }}
-                                            </div>
+                                            </label>
                                             <input
                                                 type="datetime-local"
+                                                id="wiz-group-assessment-due"
                                                 [ngModel]="newGroupAssessmentDueDate()"
                                                 (ngModelChange)="newGroupAssessmentDueDate.set($event)"
                                                 tumUiInput

src/main/webapp/app/course/manage/exercises/create-variant-modal/exercise-variant-ai-modal-wizard.component.scss

@@ -276,6 +276,8 @@
 
 .field-group {
     &__label {
+        // A <label for> rather than a <p>, so the control has an accessible name; labels are inline by default.
+        display: block;
         font-size: 0.8rem;
         font-weight: 600;
         color: var(--secondary);
@@ -565,6 +567,8 @@
 
 .new-group-settings {
     &__label {
+        // A <label for> rather than a <div>, so the control has an accessible name; labels are inline by default.
+        display: block;
         font-size: 0.7rem;
         color: var(--p-text-muted-color);
         margin-bottom: 0.2rem;

src/main/webapp/app/course/manage/overview/course-management-exercise-row.component.html

@@ -171,6 +171,7 @@
                         type="button"
                         class="btn btn-warning me-1 mb-1"
                         [ngbTooltip]="'artemisApp.exerciseManagement.action.createVariantWithAi' | artemisTranslate"
+                        [attr.aria-label]="'artemisApp.exerciseManagement.action.createVariantWithAi' | artemisTranslate"
                         (click)="aiVariantModalVisible.set(true)"
                     >
                         <fa-icon [icon]="faRobot" />

src/main/webapp/i18n/de/exerciseManagement.json

@@ -7,7 +7,7 @@
             "action": {
                 "import": "Importieren",
                 "export": "Exportieren",
-                "createVariantWithAi": "Variante erstellen"
+                "createVariantWithAi": "Variante mit KI erstellen"
             },
             "error": {
                 "onlyIndividualQuiz": "Nur Quizze im Einzelmodus können einer Aufgabengruppe hinzugefügt werden."

src/main/webapp/i18n/en/exerciseManagement.json

@@ -7,7 +7,7 @@
             "action": {
                 "import": "Import",
                 "export": "Export",
-                "createVariantWithAi": "Create Variant"
+                "createVariantWithAi": "Create Variant with AI"
             },
             "error": {
                 "onlyIndividualQuiz": "Only individual-mode quizzes can be added to an exercise group."

src/test/java/de/tum/cit/aet/artemis/hyperion/service/variants/ExerciseVariantJobServicePublishFailureTest.java

@@ -0,0 +1,82 @@
+package de.tum.cit.aet.artemis.hyperion.service.variants;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatCode;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.doThrow;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.List;
+
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import de.tum.cit.aet.artemis.account.domain.User;
+import de.tum.cit.aet.artemis.core.service.distributed.local.LocalDataProviderService;
+import de.tum.cit.aet.artemis.exercise.domain.Exercise;
+import de.tum.cit.aet.artemis.exercise.domain.ExerciseType;
+import de.tum.cit.aet.artemis.hyperion.dto.VariantGenerationRequestDTO;
+import de.tum.cit.aet.artemis.hyperion.service.websocket.HyperionWebsocketService;
+
+/**
+ * Regression test: a failed websocket publish must not undo a persisted terminal transition.
+ * <p>
+ * Every terminal transition writes the new phase first and only then publishes. When the publish threw, the
+ * exception escaped {@code complete()} into the pipeline's terminal catch, which deleted the freshly generated
+ * variant exercise and overwrote COMPLETED with FAILED — a lost notification destroying verified work.
+ * Publication is best effort: the client also polls the job endpoints, so a dropped event costs freshness only.
+ */
+class ExerciseVariantJobServicePublishFailureTest {
+
+    private static final String LOGIN = "instructor1";
+
+    private ExerciseVariantJobService jobService;
+
+    private VariantJob job;
+
+    @BeforeEach
+    void setUp() {
+        HyperionWebsocketService websocketService = mock(HyperionWebsocketService.class);
+        // The broker is unavailable / the payload cannot be converted — an unchecked throw that
+        // HyperionWebsocketService does not catch itself (it only handles Interrupted/ExecutionException).
+        doThrow(new IllegalStateException("broker unavailable")).when(websocketService).send(anyString(), anyString(), any());
+
+        jobService = new ExerciseVariantJobService(new LocalDataProviderService(), websocketService);
+        jobService.init();
+
+        Exercise exercise = mock(Exercise.class);
+        when(exercise.getId()).thenReturn(1L);
+        when(exercise.getTitle()).thenReturn("Test Exercise");
+        when(exercise.getExerciseType()).thenReturn(ExerciseType.PROGRAMMING);
+        User user = mock(User.class);
+        when(user.getLogin()).thenReturn(LOGIN);
+
+        job = jobService.startJob(user, exercise, mock(VariantGenerationRequestDTO.class));
+    }
+
+    @Test
+    void shouldKeepTheJobCompletedWhenTheDoneEventCannotBePublished() {
+        assertThatCode(() -> jobService.complete(job.getJobId(), 42L, List.of())).doesNotThrowAnyException();
+
+        VariantJob stored = jobService.getJob(job.getJobId(), LOGIN).orElseThrow();
+        assertThat(stored.getPhase()).isEqualTo(VariantJobPhase.COMPLETED);
+        // The deep link must survive: the variant exercise exists and this id is the only pointer to it.
+        assertThat(stored.getVariantExerciseId()).isEqualTo(42L);
+    }
+
+    @Test
+    void shouldStillReachTheTerminalPhaseWhenAFailureEventCannotBePublished() {
+        assertThatCode(() -> jobService.fail(job.getJobId(), "Failed in VERIFYING")).doesNotThrowAnyException();
+
+        assertThat(jobService.getJob(job.getJobId(), LOGIN).orElseThrow().getPhase()).isEqualTo(VariantJobPhase.FAILED);
+    }
+
+    @Test
+    void shouldStillReachTheTerminalPhaseWhenACancellationEventCannotBePublished() {
+        assertThatCode(() -> jobService.markCancelled(job.getJobId())).doesNotThrowAnyException();
+
+        assertThat(jobService.getJob(job.getJobId(), LOGIN).orElseThrow().getPhase()).isEqualTo(VariantJobPhase.CANCELLED);
+    }
+}