Numeric Input: Treat unrecognized simplify values as "required" (#2308)

Context: I'm seeing errors in Sentry because the perseus JSON parser doesn't
understand some of the values of `simplify` that exist in production data. This
PR ensures we accept and handle those values.

## Summary:
- I added tests to characterize the behavior of numeric input scoring for all
  the values of `simplify` that are in production.
- I updated the perseus JSON parser to convert nonstandard `simplify` values
  to equivalent ones.

Issue: none

## Test plan:

`yarn test`

Author: benchristel

Reviewers: benchristel, SonicScrewdriver, handeyeco

Required Reviewers:

Approved By: SonicScrewdriver

Checks: ✅ 8 checks were successful

Pull Request URL: #2308

Author: benchristel

.changeset/fair-ravens-guess.md

@@ -0,0 +1,5 @@
+---
+"@khanacademy/perseus-core": patch
+---
+
+Fix a bug in numeric input scoring where `simplify` value of `false` led to incorrect scoring logic being applied

packages/perseus-core/src/data-schema.ts

@@ -1206,15 +1206,19 @@ export type MathFormat =
     | "pi";
 
 export type PerseusNumericInputAnswerForm = {
-    simplify: PerseusNumericInputSimplify | null | undefined;
+    simplify: PerseusNumericInputSimplify;
     name: MathFormat;
 };
 
-export type PerseusNumericInputSimplify =
-    | "required"
-    | "correct"
-    | "enforced"
-    | "optional";
+/**
+ * Determines how unsimplified fractions are scored.
+ *
+ * - "required" means unsimplified fractions are considered invalid input, and
+ *   the learner can try again.
+ * - "enforced" means unsimplified fractions are marked incorrect.
+ * - "optional" means unsimplified fractions are accepted.
+ */
+export type PerseusNumericInputSimplify = "required" | "enforced" | "optional";
 
 export type PerseusNumericInputWidgetOptions = {
     // A list of all the possible correct and incorrect answers
@@ -1249,7 +1253,7 @@ export type PerseusNumericInputAnswer = {
     // NOTE: perseus_data.go says this is non-nullable even though we handle null values.
     maxError: number | null | undefined;
     // Unsimplified answers are Ungraded, Accepted, or Wrong. Options: "required", "correct", or "enforced"
-    simplify: PerseusNumericInputSimplify | null | undefined;
+    simplify: PerseusNumericInputSimplify;
 };
 
 export type PerseusNumberLineWidgetOptions = {

packages/perseus-core/src/parse-perseus-json/perseus-parsers/numeric-input-widget.test.ts

@@ -0,0 +1,47 @@
+import {anyFailure} from "../general-purpose-parsers/test-helpers";
+import {parse} from "../parse";
+import {success} from "../result";
+
+import {parseSimplify} from "./numeric-input-widget";
+
+describe("parseSimplify", () => {
+    it(`preserves "required"`, () => {
+        expect(parse("required", parseSimplify)).toEqual(success("required"));
+    });
+
+    it(`preserves "enforced"`, () => {
+        expect(parse("enforced", parseSimplify)).toEqual(success("enforced"));
+    });
+
+    it(`preserves "optional"`, () => {
+        expect(parse("optional", parseSimplify)).toEqual(success("optional"));
+    });
+
+    it(`converts null to "required"`, () => {
+        expect(parse(null, parseSimplify)).toEqual(success("required"));
+    });
+
+    it(`converts undefined to "required"`, () => {
+        expect(parse(undefined, parseSimplify)).toEqual(success("required"));
+    });
+
+    it(`converts true to "required"`, () => {
+        expect(parse(true, parseSimplify)).toEqual(success("required"));
+    });
+
+    it(`converts false to "required"`, () => {
+        expect(parse(false, parseSimplify)).toEqual(success("required"));
+    });
+
+    it(`converts "accepted" to "required"`, () => {
+        expect(parse("accepted", parseSimplify)).toEqual(success("required"));
+    });
+
+    it(`converts "correct" to "required"`, () => {
+        expect(parse("correct", parseSimplify)).toEqual(success("required"));
+    });
+
+    it(`rejects an arbitrary string`, () => {
+        expect(parse("foobar", parseSimplify)).toEqual(anyFailure);
+    });
+});

packages/perseus-core/src/parse-perseus-json/perseus-parsers/numeric-input-widget.ts

@@ -16,7 +16,10 @@ import {defaulted} from "../general-purpose-parsers/defaulted";
 
 import {parseWidget} from "./widget";
 
-import type {NumericInputWidget} from "../../data-schema";
+import type {
+    NumericInputWidget,
+    PerseusNumericInputSimplify,
+} from "../../data-schema";
 import type {Parser} from "../parser-types";
 
 const parseMathFormat = enumeration(
@@ -29,12 +32,41 @@ const parseMathFormat = enumeration(
     "pi",
 );
 
-const parseSimplify = enumeration(
-    "required",
-    "correct",
-    "enforced",
-    "optional",
-);
+export const parseSimplify = pipeParsers(
+    union(constant(null))
+        .or(constant(undefined))
+        .or(boolean)
+        .or(constant("required"))
+        .or(constant("correct"))
+        .or(constant("enforced"))
+        .or(constant("optional"))
+        .or(constant("accepted")).parser,
+).then(convert(deprecatedSimplifyValuesToRequired)).parser;
+
+function deprecatedSimplifyValuesToRequired(
+    simplify:
+        | "required"
+        | "correct"
+        | "enforced"
+        | "optional"
+        | "accepted"
+        | null
+        | undefined
+        | boolean,
+): PerseusNumericInputSimplify {
+    switch (simplify) {
+        case "enforced":
+        case "required":
+        case "optional":
+            return simplify;
+        // NOTE(benchristel): "accepted", "correct", true, false, undefined, and
+        // null are all treated the same as "required" during scoring, so we
+        // convert them to "required" here to preserve behavior. See the tests
+        // in score-numeric-input.test.ts
+        default:
+            return "required";
+    }
+}
 
 export const parseNumericInputWidget: Parser<NumericInputWidget> = parseWidget(
     constant("numeric-input"),
@@ -53,20 +85,7 @@ export const parseNumericInputWidget: Parser<NumericInputWidget> = parseWidget(
                 // TODO(benchristel): simplify should never be a boolean, but we
                 // have some content where it is anyway. If we ever backfill
                 // the data, we should simplify `simplify`.
-                simplify: optional(
-                    nullable(
-                        union(parseSimplify).or(
-                            pipeParsers(boolean).then(
-                                convert((value) => {
-                                    if (typeof value === "boolean") {
-                                        return value ? "required" : "optional";
-                                    }
-                                    return value;
-                                }),
-                            ).parser,
-                        ).parser,
-                    ),
-                ),
+                simplify: parseSimplify,
             }),
         ),
         labelText: optional(string),
@@ -78,16 +97,7 @@ export const parseNumericInputWidget: Parser<NumericInputWidget> = parseWidget(
             array(
                 object({
                     name: parseMathFormat,
-                    simplify: optional(
-                        nullable(
-                            enumeration(
-                                "required",
-                                "correct",
-                                "enforced",
-                                "optional",
-                            ),
-                        ),
-                    ),
+                    simplify: parseSimplify,
                 }),
             ),
         ),