From 898de758ba9f1d2e071b092bcdc93fd6d6e48406 Mon Sep 17 00:00:00 2001
From: Jakub Zych <jakub@keios.eu>
Date: Mon, 9 Nov 2026 22:53:17 +0100
Subject: [PATCH] fix: mark an answer failed when its last grading write fails

WR-07: when storing the grader's score fails on the last River attempt,
record the answer as failed instead of leaving it pending with no job.
The terminal failure write runs on a context detached from the job's,
so a timed-out or cancelled job still records it.
---
 classes/grading.go | 38 +++++++++++++++++++++++++++++++-------
 1 file changed, 31 insertions(+), 7 deletions(-)

diff --git a/classes/grading.go b/classes/grading.go
index 9a9dc99..f7686c6 100644
--- a/classes/grading.go
+++ b/classes/grading.go
@@ -189,11 +189,12 @@ func fireAfterCommit[T any](ctx context.Context, s *Service, tx *gorm.DB, name s
 // A missing answer, one that is no longer pending (graded, failed or
 // reviewed by the owner), one whose grading_seq differs from seq (the
 // student wrote a newer text) and one whose question is no longer open are
-// left alone, as is an answer whose quiz has no registered grader. Each run counts an attempt for the current
-// text first. A grader error, or a score ClampOpenPoints refuses, is
-// returned for River to retry while fewer than GradeMaxAttempts attempts
-// were made; the last one marks the answer failed with the error and
-// returns nil (D-17). A score is stored as graded with points clamped to
+// left alone, as is an answer whose quiz has no registered grader. Each
+// run counts an attempt for the current text first. A grader error, or a
+// score ClampOpenPoints refuses, is returned for River to retry while
+// fewer than GradeMaxAttempts attempts were made; the last one marks the
+// answer failed with the error and returns nil (D-17), as does an error
+// storing the score on the last attempt. A score is stored as graded with points clamped to
 // 0..question points in steps of 0.5, the attempt's score is recalculated
 // when it is completed, and AnswerGradedEvent fires after the commit.
 func (s *Service) GradeAnswer(ctx context.Context, answerID uint, seq int) error {
@@ -266,9 +267,32 @@ WHERE id = ? AND grading_status = ? AND grading_seq = ? RETURNING grading_attemp
 	case GradingRetry:
 		return gradeErr
 	case GradingFail:
-		return s.failGrading(ctx, row.ID, attempt.ID, seq, grader.Code(), gradeErr)
+		return s.failLast(ctx, row.ID, attempt.ID, seq, grader.Code(), gradeErr)
 	case GradingStore:
-		return s.storeGrade(ctx, row.ID, seq, &attempt, question, grader.Code(), points, res.Feedback)
+		err := s.storeGrade(ctx, row.ID, seq, &attempt, question, grader.Code(), points, res.Feedback)
+		if err != nil && attempts >= GradeMaxAttempts {
+			// River will not run the job again: without this the answer
+			// would stay pending with no job (D-17).
+			return s.failLast(ctx, row.ID, attempt.ID, seq, grader.Code(), err)
+		}
+		return err
+	}
+	return nil
+}
+
+// failLastTimeout bounds the write that marks an answer failed after its
+// last attempt.
+const failLastTimeout = 30 * time.Second
+
+// failLast marks an answer failed after its last grading attempt on a
+// context detached from the job's, so a job that timed out or was
+// cancelled still records the failure. If even that write fails, both
+// errors are returned and the answer stays pending for a regrade.
+func (s *Service) failLast(ctx context.Context, answerID, attemptID uint, seq int, code string, cause error) error {
+	fctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), failLastTimeout)
+	defer cancel()
+	if err := s.failGrading(fctx, answerID, attemptID, seq, code, cause); err != nil {
+		return errors.Join(cause, err)
 	}
 	return nil
 }