Skip to content

Commit

Permalink
BUGFIX - [Numeric Input] - Check for wrong answers when scoring (#1743)
Browse files Browse the repository at this point in the history
## 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: #1743
  • Loading branch information
mark-fitzgerald authored Oct 28, 2024
1 parent f3139ed commit 5ea5d59
Show file tree
Hide file tree
Showing 3 changed files with 137 additions and 51 deletions.
5 changes: 5 additions & 0 deletions .changeset/spotty-numbers-arrive.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@khanacademy/perseus": patch
---

BUGFIX - [Numeric Input] - Check for wrong answers when scoring
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 5ea5d59

Please sign in to comment.