From 5ea5d592755bd5b2889547718fc39523e5595ea1 Mon Sep 17 00:00:00 2001 From: Mark Fitzgerald <13896410+mark-fitzgerald@users.noreply.github.com> Date: Mon, 28 Oct 2024 07:48:19 -0700 Subject: [PATCH] BUGFIX - [Numeric Input] - Check for wrong answers when scoring (#1743) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary: On occasion, answer options within the Numeric Input can overlap. In cases where the wrong/ungraded answer comes before the correct answer, it is important to honor this order when validating a learner's answer. Currently, the Numeric Input widget checks all correct answers for a match before looking at the remaining answer options. This means that an incorrect answer would be marked as correct. This bugfix changes the validation logic to look for the first match regardless of the correctness of the answer itself, and then reports the validation based upon the match. Issue: LEMS-2238 ## Test plan: **No Storybook option exists for testing validation of a widget - [must be tested in a ZND](https://prod-znd-241016-markfitz-m01.khanacademy.org)** 1. Open a Test Everything exercise ([Numeric Input](https://prod-znd-241016-markfitz-m01.khanacademy.org/internal-courses/test-everything/test-everything-2-without-mastery/te-numeric-input/e/numeric-input-exercise) may be helpful) * Make sure to be logged in 1. Edit the exercise ![Click to edit](https://github.com/user-attachments/assets/59fa66dc-3cd2-45f1-8f5a-ab470997cf93) 1. Add a new item to the exercise ![Click to add](https://github.com/user-attachments/assets/f4e3f096-6ea0-4258-bcba-679783d671a6) 1. Add a Numeric Input widget to the item ![Add widget](https://github.com/user-attachments/assets/6359fd1a-c48c-44f9-b1b4-f42327b87be6) 1. Enter an answer of "4" and mark it wrong by clicking on the green "correct" indicator ![Add wrong answer](https://github.com/user-attachments/assets/2d1cd161-da8a-44c4-898a-8193d01fe1da) 1. Add another answer ![Add new answer](https://github.com/user-attachments/assets/abfdd21e-2d59-40df-bfaa-2b2e8680310f) 1. Enter an answer of "10 +/- 10" ![Add correct answer](https://github.com/user-attachments/assets/20ebda59-e066-4e6b-a3a6-b1e4ff8407e5) 1. Test the widget in "Preview" ![Test widget](https://github.com/user-attachments/assets/06f91487-1edd-41fe-ab1c-b43911c5d17b) * Enter the number "4" and click "Check" button - button text should change to "Try again" * Enter a different number (like "5") and click the "Check"/"Try again" button - confetti and button text changes to "Done" Author: mark-fitzgerald Reviewers: SonicScrewdriver Required Reviewers: Approved By: SonicScrewdriver Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ gerald Pull Request URL: https://github.com/Khan/perseus/pull/1743 --- .changeset/spotty-numbers-arrive.md | 5 + .../numeric-input-validator.test.ts | 94 +++++++++++++++++++ .../numeric-input/numeric-input-validator.ts | 89 ++++++++---------- 3 files changed, 137 insertions(+), 51 deletions(-) create mode 100644 .changeset/spotty-numbers-arrive.md diff --git a/.changeset/spotty-numbers-arrive.md b/.changeset/spotty-numbers-arrive.md new file mode 100644 index 0000000000..af5c0da5df --- /dev/null +++ b/.changeset/spotty-numbers-arrive.md @@ -0,0 +1,5 @@ +--- +"@khanacademy/perseus": patch +--- + +BUGFIX - [Numeric Input] - Check for wrong answers when scoring diff --git a/packages/perseus/src/widgets/numeric-input/numeric-input-validator.test.ts b/packages/perseus/src/widgets/numeric-input/numeric-input-validator.test.ts index 7fa891e125..cd3f905bf9 100644 --- a/packages/perseus/src/widgets/numeric-input/numeric-input-validator.test.ts +++ b/packages/perseus/src/widgets/numeric-input/numeric-input-validator.test.ts @@ -222,6 +222,100 @@ describe("static function validate", () => { expect(score).toHaveBeenAnsweredCorrectly(); }); + + it("respects the order of answer options when scoring", () => { + // Arrange + const rubric: PerseusNumericInputRubric = { + answers: [ + // "4" is a wrong answer + { + value: 4, + status: "wrong", + maxError: 0, + simplify: "", + strict: false, + message: "", + }, + // Any number between "0" and "20" is correct, except for "4" + { + value: 10, + status: "correct", + maxError: 10, + simplify: "", + strict: false, + message: "", + }, + ], + labelText: "", + size: "normal", + static: false, + coefficient: true, + }; + + // Act - "wrong" + const wrongInput = { + currentValue: "4", + } as const; + let score = numericInputValidator(wrongInput, rubric, mockStrings); + + // Assert - "wrong" + expect(score).toHaveBeenAnsweredIncorrectly(); + + // Act - "correct" + const correctInput = { + currentValue: "14", + } as const; + score = numericInputValidator(correctInput, rubric, mockStrings); + + // Assert - "correct" + expect(score).toHaveBeenAnsweredCorrectly(); + }); + + it("defaults to 1 or -1 when user input is empty/incomplete", () => { + // Arrange + const rubric: PerseusNumericInputRubric = { + answers: [ + { + value: 1, + status: "correct", + maxError: 0, + simplify: "", + strict: false, + message: "", + }, + { + value: -1, + status: "correct", + maxError: 0, + simplify: "", + strict: false, + message: "", + }, + ], + labelText: "", + size: "normal", + static: false, + coefficient: true, + }; + + // Act - "empty" + const emptyInput = { + currentValue: "", + } as const; + let score = numericInputValidator(emptyInput, rubric, mockStrings); + + // Assert - "empty" + expect(score).toHaveBeenAnsweredCorrectly(); + + // Act - "incomplete" + const incompleteInput = { + currentValue: "-", + } as const; + score = numericInputValidator(incompleteInput, rubric, mockStrings); + + // Assert - "incomplete" + expect(score).toHaveBeenAnsweredCorrectly(); + }); }); describe("maybeParsePercentInput utility function", () => { diff --git a/packages/perseus/src/widgets/numeric-input/numeric-input-validator.ts b/packages/perseus/src/widgets/numeric-input/numeric-input-validator.ts index 889b5dac5b..280fd2c3b7 100644 --- a/packages/perseus/src/widgets/numeric-input/numeric-input-validator.ts +++ b/packages/perseus/src/widgets/numeric-input/numeric-input-validator.ts @@ -4,6 +4,7 @@ import KhanAnswerTypes from "../../util/answer-types"; import type {MathFormat, PerseusNumericInputAnswer} from "../../perseus-types"; import type {PerseusStrings} from "../../strings"; import type {PerseusScore} from "../../types"; +import type {Score} from "../../util/answer-types"; import type { PerseusNumericInputRubric, PerseusNumericInputUserInput, @@ -107,62 +108,48 @@ function numericInputValidator( // We may have received TeX; try to parse it before grading. // If `currentValue` is not TeX, this should be a no-op. const currentValue = ParseTex(userInput.currentValue); - const correctAnswers = rubric.answers.filter( - (answer) => answer.status === "correct", - ); - - const normalizedAnswerExpected = correctAnswers.every( - (answer) => Math.abs(answer.value) <= 1, - ); - - // Look through all correct answers for one that matches either - // precisely or approximately and return the appropriate message: - // - if precise, return the message that the answer came with - // - if it needs to be simplified, etc., show that message - let result = correctAnswers + + const normalizedAnswerExpected = rubric.answers + .filter((answer) => answer.status === "correct") + .every((answer) => Math.abs(answer.value) <= 1); + + // The coefficient is an attribute of the widget + let localValue: string | number = currentValue; + if (rubric.coefficient) { + if (!localValue) { + localValue = 1; + } else if (localValue === "-") { + localValue = -1; + } + } + const matchedAnswer: + | (PerseusNumericInputAnswer & {score: Score}) + | undefined = rubric.answers .map((answer) => { - // The coefficient is an attribute of the widget - let localValue: string | number = currentValue; - if (rubric.coefficient) { - if (!localValue) { - localValue = 1; - } else if (localValue === "-") { - localValue = -1; - } - } - const validate = createValidator(answer); - return validate( + const validateFn = createValidator(answer); + const score = validateFn( maybeParsePercentInput(localValue, normalizedAnswerExpected), ); + return {...answer, score}; }) - .find((match) => match.correct || match.empty); - - if (!result) { - // Otherwise, if the guess is not correct - const otherAnswers = [].concat( - // @ts-expect-error - TS2769 - No overload matches this call. - rubric.answers.filter((answer) => answer.status === "ungraded"), - rubric.answers.filter((answer) => answer.status === "wrong"), - ); - - // Look through all other answers and if one matches either - // precisely or approximately return the answer's message - const match = otherAnswers.find((answer) => { - const validate = createValidator(answer); - return validate( - maybeParsePercentInput(currentValue, normalizedAnswerExpected), - ).correct; + .find((answer) => { + // NOTE: "answer.score.correct" indicates a match via the validate function. + // It does NOT indicate that the answer itself is correct. + return ( + answer.score.correct || + (answer.status === "correct" && answer.score.empty) + ); }); - result = { - // @ts-expect-error - TS2339 - Property 'status' does not exist on type 'never'. - empty: match ? match.status === "ungraded" : false, - // @ts-expect-error - TS2339 - Property 'status' does not exist on type 'never'. - correct: match ? match.status === "correct" : false, - // @ts-expect-error - TS2339 - Property 'message' does not exist on type 'never'. - message: match ? match.message : null, - guess: currentValue, - }; - } + + const result: Score = + matchedAnswer?.status === "correct" + ? matchedAnswer.score + : { + empty: matchedAnswer?.status === "ungraded", + correct: matchedAnswer?.status === "correct", + message: matchedAnswer?.message ?? null, + guess: localValue, + }; // TODO(eater): Seems silly to translate result to this // invalid/points thing and immediately translate it