From adc02d7e0c39c1ff17174fb170c9fe237c002987 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Mon, 9 Nov 2026 22:46:41 +0100 Subject: [PATCH] fix: withhold grader feedback that repeats the rubric WR-01: a deterministic server-side guard behind the grader's prompt. storeGrade drops feedback sharing a run of six words with the rubric (runs also in the question or model answer do not count) and notes it for the owner in grading_error. --- classes/grading.go | 80 +++++++++++++++++++++++++++++++++++++++-- classes/grading_test.go | 31 ++++++++++++++++ mcp_reviews.go | 2 +- 3 files changed, 110 insertions(+), 3 deletions(-) diff --git a/classes/grading.go b/classes/grading.go index 4d0de23..f9a89e7 100644 --- a/classes/grading.go +++ b/classes/grading.go @@ -5,6 +5,7 @@ import ( "errors" "strings" "time" + "unicode" "unicode/utf8" "git.golem15.com/golem15/sm-quizzes-plugin/events" @@ -306,7 +307,9 @@ func (s *Service) failGrading(ctx context.Context, answerID, attemptID uint, seq } // storeGrade stores a grader's clamped score as graded, recalculates a -// completed attempt and fires AnswerGradedEvent after the commit. +// completed attempt and fires AnswerGradedEvent after the commit. Feedback +// that repeats the rubric (LeaksRubric) is dropped, for the student and the +// event alike, and noted for the owner in grading_error. func (s *Service) storeGrade(ctx context.Context, answerID uint, seq int, attempt *models.Attempt, question models.Question, code string, points float64, feedback string) error { return lagoon.Transaction(ctx, s.DB, func(ctx context.Context, tx *gorm.DB) error { tx = tx.WithContext(ctx) @@ -320,6 +323,11 @@ func (s *Service) storeGrade(ctx context.Context, answerID uint, seq int, attemp return err } feedback = cutRunes(strings.TrimSpace(feedback), maxGraderFeedback) + var note any + if LeaksRubric(feedback, deref(question.Rubric), question.Text, deref(question.AnswerText)) { + pluginLog.Warn("quizzes: grader feedback withheld, it repeats the rubric", "answer_id", answerID, "grader", code) + feedback, note = "", feedbackWithheldNote + } var fb any if feedback != "" { fb = feedback @@ -332,7 +340,7 @@ func (s *Service) storeGrade(ctx context.Context, answerID uint, seq int, attemp "points": points, "is_correct": OpenCorrect(points, question.Points), "grader_feedback": fb, - "grading_error": nil, + "grading_error": note, "graded_at": now, "updated_at": now, }).Error; err != nil { @@ -356,6 +364,74 @@ func (s *Service) storeGrade(ctx context.Context, answerID uint, seq int, attemp }) } +// feedbackWithheldNote is the owner's note on a graded answer whose +// feedback LeaksRubric dropped; students never see grading_error. +const feedbackWithheldNote = "feedback withheld: it repeated the rubric" + +// Rubric leak detection: a run of rubricRunWords consecutive words shared +// by the feedback and the rubric counts as quoting it. A rubric shorter +// than that counts when the feedback holds it whole, unless it has fewer +// than minRubricWords words (too short to tell from ordinary wording). +const ( + rubricRunWords = 6 + minRubricWords = 4 +) + +// LeaksRubric reports whether a grader's feedback repeats the owner-only +// rubric (D-01): a run of words of the rubric, compared case-insensitively +// word by word, that does not also occur in one of the public texts (the +// question and the model answer, which the student is shown anyway). It is +// a deterministic guard behind the grader's prompt, so a model talked into +// listing the rubric never puts it in a student response verbatim. +func LeaksRubric(feedback, rubric string, public ...string) bool { + rw := words(rubric) + if len(rw) < minRubricWords { + return false + } + n := min(rubricRunWords, len(rw)) + fw := words(feedback) + if len(fw) < n { + return false + } + skip := map[string]bool{} + for _, p := range public { + for g := range ngrams(words(p), n) { + skip[g] = true + } + } + secret := map[string]bool{} + for g := range ngrams(rw, n) { + if !skip[g] { + secret[g] = true + } + } + if len(secret) == 0 { + return false + } + for g := range ngrams(fw, n) { + if secret[g] { + return true + } + } + return false +} + +// words splits s into lower-case words of letters and digits. +func words(s string) []string { + return strings.FieldsFunc(strings.ToLower(s), func(r rune) bool { + return !unicode.IsLetter(r) && !unicode.IsNumber(r) + }) +} + +// ngrams is the set of runs of n consecutive words of w, joined by spaces. +func ngrams(w []string, n int) map[string]bool { + out := map[string]bool{} + for i := 0; i+n <= len(w); i++ { + out[strings.Join(w[i:i+n], " ")] = true + } + return out +} + // cutRunes keeps at most n runes of s. func cutRunes(s string, n int) string { if utf8.RuneCountInString(s) <= n { diff --git a/classes/grading_test.go b/classes/grading_test.go index 23d426a..758716b 100644 --- a/classes/grading_test.go +++ b/classes/grading_test.go @@ -72,3 +72,34 @@ func TestNextGrading(t *testing.T) { t.Errorf("GradeMaxAttempts = %d, want 3", classes.GradeMaxAttempts) } } + +func TestLeaksRubric(t *testing.T) { + rubric := "Pełne punkty: wymienia trzy przyczyny upadku Republiki Weimarskiej, w tym kryzys gospodarczy z 1929 roku." + question := "Wyjaśnij, dlaczego upadła Republika Weimarska." + model := "Republika upadła przez kryzys gospodarczy z 1929 roku i słabość instytucji." + cases := []struct { + name string + feedback string + want bool + }{ + {"own words", "Dobra odpowiedź, ale zabrakło trzeciej przyczyny i dat.", false}, + {"verbatim run", "Oczekiwano, że odpowiedź WYMIENIA trzy przyczyny upadku Republiki Weimarskiej.", true}, + {"punctuation and case ignored", "pełne punkty - wymienia, trzy; przyczyny upadku republiki!", true}, + {"run also in the model answer", "Brakuje wzmianki o tym, że kryzys gospodarczy z 1929 roku i słabość instytucji zaszkodziły.", false}, + {"empty feedback", "", false}, + } + for _, tc := range cases { + if got := classes.LeaksRubric(tc.feedback, rubric, question, model); got != tc.want { + t.Errorf("%s: LeaksRubric = %v, want %v", tc.name, got, tc.want) + } + } + if classes.LeaksRubric("Za mało szczegółów.", "", question) { + t.Error("an empty rubric leaks") + } + if classes.LeaksRubric("Wymień trzy przyczyny.", "trzy przyczyny", question) { + t.Error("a rubric under the minimum length leaks") + } + if !classes.LeaksRubric("Wymagano: podać daty, nazwiska i skutki. Zabrakło dat.", "Podać daty, nazwiska i skutki.") { + t.Error("a short rubric quoted whole does not leak") + } +} diff --git a/mcp_reviews.go b/mcp_reviews.go index 4e4a413..83d2ca8 100644 --- a/mcp_reviews.go +++ b/mcp_reviews.go @@ -58,7 +58,7 @@ func reviewTools() []pact.MCPTool { { Name: "review_show", Title: "Show one open answer for review", - Description: "Shows one open answer for review, owner view with the solution: question {id, text, points, answer_text (the model answer), rubric (the grading rules), max_length}, the student's full text, quiz, attempt, student, status, grader (the grader code), grader_points and grader_feedback (the grader's score and note, final only once status is reviewed), grading_error (why the grader failed), grading_attempts, points (what counts in the attempt's score now), is_correct, review_comment (shown to the student), reviewed_at and reviewed_by {user_id, admin_id}. Never pass the rubric or the model answer to a student." + needsRead, + Description: "Shows one open answer for review, owner view with the solution: question {id, text, points, answer_text (the model answer), rubric (the grading rules), max_length}, the student's full text, quiz, attempt, student, status, grader (the grader code), grader_points and grader_feedback (the grader's score and note, final only once status is reviewed), grading_error (why the grader failed, or that its feedback was withheld for repeating the rubric), grading_attempts, points (what counts in the attempt's score now), is_correct, review_comment (shown to the student), reviewed_at and reviewed_by {user_id, admin_id}. Never pass the rubric or the model answer to a student." + needsRead, Method: http.MethodGet, Path: ReviewPrefix + "/{id}", Input: reviewAnswerInput{},