SSS: Hook emptyWidgets() up to validation functions (#2000)

## Summary: This PR begins connecting the validation functions that have been built to the existing `emptyWidgets()` function that already exists on the `Renderer` (as well as `emptyWidgesFunctional()`). This function powers the `APIOptions.answerableCallback` which is what the exercise chrome using Perseus uses to detect if a question is at a point where it can be scored. Note that some widget's "emptiness" checks depend on scoring data and so those checks are not included in the empty checking now. Issue: LEMS-2561 ## Test plan: yarn test yarn typecheck Author: jeremywiebe Reviewers: jeremywiebe, handeyeco, Myranae Required Reviewers: Approved By: handeyeco Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x) Pull Request URL: #2000
Khan · Dec 14, 2024 · 0db68d2 · 0db68d2
1 parent 99cd254
commit 0db68d2
Show file tree

Hide file tree

Showing 20 changed files with 243 additions and 49 deletions.
diff --git a/.changeset/quiet-adults-look.md b/.changeset/quiet-adults-look.md
@@ -0,0 +1,5 @@
+---
+"@khanacademy/perseus": minor
+---
+
+Change empty widgets check in Renderer to depend only on data available (and not on scoring data)
diff --git a/packages/perseus/src/__testdata__/renderer.testdata.ts b/packages/perseus/src/__testdata__/renderer.testdata.ts
@@ -1,11 +1,29 @@
 import type {
     DropdownWidget,
+    ExpressionWidget,
     ImageWidget,
     NumericInputWidget,
     PerseusRenderer,
 } from "../perseus-types";
 import type {RenderProps} from "../widgets/radio";
 
+export const expressionWidget: ExpressionWidget = {
+    type: "expression",
+    options: {
+        answerForms: [
+            {
+                considered: "correct",
+                form: true,
+                simplify: true,
+                value: "1.0",
+            },
+        ],
+        buttonSets: ["basic"],
+        functions: [],
+        times: true,
+    },
+};
+
 export const dropdownWidget: DropdownWidget = {
     type: "dropdown",
     alignment: "default",
@@ -96,6 +114,12 @@ export const question2: PerseusRenderer = {
     widgets: {"numeric-input 1": numericInputWidget},
 };
 
+export const question3: PerseusRenderer = {
+    content: "Enter $1.0$ in the input field: [[\u2603 expression 1]]\n\n\n\n",
+    images: {},
+    widgets: {"expression 1": expressionWidget},
+};
+
 export const definitionItem: PerseusRenderer = {
     content:
         "Mock widgets ==> [[\u2603 definition 1]] [[\u2603 definition 2]] [[\u2603 definition 3]]",

diff --git a/packages/perseus/src/__tests__/renderer.test.tsx b/packages/perseus/src/__tests__/renderer.test.tsx
@@ -15,6 +15,7 @@ import {
     definitionItem,
     mockedRandomItem,
     mockedShuffledRadioProps,
+    question3,
 } from "../__testdata__/renderer.testdata";
 import * as Dependencies from "../dependencies";
 import {registerWidget} from "../widgets";
@@ -1605,35 +1606,36 @@ describe("renderer", () => {
         it("should return all empty widgets", async () => {
             // Arrange
             const {renderer} = renderQuestion({
-                ...question2,
+                ...question3,
                 content:
-                    "Input 1: [[☃ numeric-input 1]]\n\n" +
-                    "Input 2: [[☃ numeric-input 2]]",
+                    "Input 1: [[☃ expression 1]]\n\n" +
+                    "Input 2: [[☃ expression 2]]",
                 widgets: {
-                    ...question2.widgets,
-                    "numeric-input 2": question2.widgets["numeric-input 1"],
+                    ...question3.widgets,
+                    "expression 2": question3.widgets["expression 1"],
                 },
             });
             await userEvent.type(screen.getAllByRole("textbox")[0], "150");
+            act(() => jest.runOnlyPendingTimers());
 
             // Act
             const emptyWidgets = renderer.emptyWidgets();
 
             // Assert
-            expect(emptyWidgets).toStrictEqual(["numeric-input 2"]);
+            expect(emptyWidgets).toStrictEqual(["expression 2"]);
         });
 
         it("should not return static widgets even if empty", () => {
             // Arrange
             const {renderer} = renderQuestion({
-                ...question2,
+                ...question3,
                 content:
-                    "Input 1: [[☃ numeric-input 1]]\n\n" +
-                    "Input 2: [[☃ numeric-input 2]]",
+                    "Input 1: [[☃ expression 1]]\n\n" +
+                    "Input 2: [[☃ expression 2]]",
                 widgets: {
-                    ...question2.widgets,
-                    "numeric-input 2": {
-                        ...question2.widgets["numeric-input 1"],
+                    ...question3.widgets,
+                    "expression 2": {
+                        ...question3.widgets["expression 1"],
                         static: true,
                     },
                 },
@@ -1643,7 +1645,7 @@ describe("renderer", () => {
             const emptyWidgets = renderer.emptyWidgets();
 
             // Assert
-            expect(emptyWidgets).toStrictEqual(["numeric-input 1"]);
+            expect(emptyWidgets).toStrictEqual(["expression 1"]);
         });
 
         it("should return widget ID for group with empty widget", () => {
@@ -1663,7 +1665,7 @@ describe("renderer", () => {
                 JSON.stringify(simpleGroupQuestion),
             );
             simpleGroupQuestionCopy.widgets["group 1"].options.widgets[
-                "numeric-input 1"
+                "expression 1"
             ].static = true;
             const {renderer} = renderQuestion(simpleGroupQuestionCopy);
 
@@ -1678,6 +1680,7 @@ describe("renderer", () => {
             // Arrange
             const {renderer} = renderQuestion(simpleGroupQuestion);
             await userEvent.type(screen.getByRole("textbox"), "99");
+            act(() => jest.runOnlyPendingTimers());
 
             // Act
             const emptyWidgets = renderer.emptyWidgets();

diff --git a/packages/perseus/src/renderer-util.ts b/packages/perseus/src/renderer-util.ts
@@ -1,12 +1,20 @@
 import {mapObject} from "./interactive2/objective_";
 import {scoreIsEmpty, flattenScores} from "./util/scoring";
 import {getWidgetIdsFromContent} from "./widget-type-utils";
-import {getWidgetScorer, upgradeWidgetInfoToLatestVersion} from "./widgets";
+import {
+    getWidgetScorer,
+    getWidgetValidator,
+    upgradeWidgetInfoToLatestVersion,
+} from "./widgets";
 
 import type {PerseusRenderer, PerseusWidgetsMap} from "./perseus-types";
 import type {PerseusStrings} from "./strings";
 import type {PerseusScore} from "./types";
-import type {UserInput, UserInputMap} from "./validation.types";
+import type {
+    UserInput,
+    UserInputMap,
+    ValidationDataMap,
+} from "./validation.types";
 
 export function getUpgradedWidgetOptions(
     oldWidgetOptions: PerseusWidgetsMap,
@@ -33,31 +41,31 @@ export function getUpgradedWidgetOptions(
     });
 }
 
+/**
+ * Checks the given user input to see if any answerable widgets have not been
+ * "filled in" (ie. if they're empty). Another way to think about this
+ * function is that its a check to see if we can score the provided input.
+ */
 export function emptyWidgetsFunctional(
-    widgets: PerseusWidgetsMap,
+    widgets: ValidationDataMap,
     // This is a port of old code, I'm not sure why
     // we need widgetIds vs the keys of the widgets object
     widgetIds: Array<string>,
     userInputMap: UserInputMap,
     strings: PerseusStrings,
     locale: string,
 ): ReadonlyArray<string> {
-    const upgradedWidgets = getUpgradedWidgetOptions(widgets);
-
     return widgetIds.filter((id) => {
-        const widget = upgradedWidgets[id];
-        if (!widget || widget.static) {
+        const widget = widgets[id];
+        if (!widget || widget.static === true) {
             // Static widgets shouldn't count as empty
             return false;
         }
 
-        const scorer = getWidgetScorer(widget.type);
-        const score = scorer?.(
-            userInputMap[id] as UserInput,
-            widget.options,
-            strings,
-            locale,
-        );
+        const validator = getWidgetValidator(widget.type);
+        const userInput = userInputMap[id];
+        const validationData = widget.options;
+        const score = validator?.(userInput, validationData, strings, locale);
 
         if (score) {
             return scoreIsEmpty(score);

diff --git a/packages/perseus/src/types.ts b/packages/perseus/src/types.ts
@@ -13,6 +13,7 @@ import type {
     UserInput,
     UserInputArray,
     UserInputMap,
+    ValidationData,
 } from "./validation.types";
 import type {WidgetPromptJSON} from "./widget-ai-utils/prompt-types";
 import type {KeypadAPI} from "@khanacademy/math-input";
@@ -578,6 +579,13 @@ export type WidgetTransform = (
 
 export type ValidationResult = Extract<PerseusScore, {type: "invalid"}> | null;
 
+export type WidgetValidatorFunction = (
+    userInput: UserInput,
+    validationData: ValidationData,
+    strings: PerseusStrings,
+    locale: string,
+) => ValidationResult;
+
 export type WidgetScorerFunction = (
     // The user data needed to score
     userInput: UserInput,
@@ -632,6 +640,14 @@ export type WidgetExports<
      */
     staticTransform?: WidgetTransform; // this is a function of some sort,
 
+    /**
+     * Validates the learner's guess to check if it's sufficient for scoring.
+     * Typically, this is basically an "emptiness" check, but for some widgets
+     * such as `interactive-graph` it is a check that the learner has made any
+     * edits (ie. the widget is not in it's origin state).
+     */
+    validator?: WidgetValidatorFunction;
+
     /**
      * A function that scores user input (the guess) for the widget.
      */

diff --git a/packages/perseus/src/validation.types.ts b/packages/perseus/src/validation.types.ts
@@ -90,6 +90,7 @@ export type PerseusExpressionRubric = {
 export type PerseusExpressionUserInput = string;
 
 export type PerseusGroupRubric = PerseusGroupWidgetOptions;
+export type PerseusGroupValidationData = {widgets: ValidationDataMap};
 export type PerseusGroupUserInput = UserInputMap;
 
 export type PerseusGradedGroupRubric = PerseusGradedGroupWidgetOptions;
@@ -264,6 +265,7 @@ export type UserInput =
     | PerseusDropdownUserInput
     | PerseusExpressionUserInput
     | PerseusGrapherUserInput
+    | PerseusGroupUserInput
     | PerseusIFrameUserInput
     | PerseusInputNumberUserInput
     | PerseusInteractiveGraphUserInput
@@ -278,11 +280,73 @@ export type UserInput =
     | PerseusSorterUserInput
     | PerseusTableUserInput;
 
-export type UserInputMap = {[widgetId: string]: UserInput | UserInputMap};
+export type UserInputMap = {[widgetId: string]: UserInput};
 
 /**
  * deprecated prefer using UserInputMap
  */
 export type UserInputArray = ReadonlyArray<
     UserInputArray | UserInput | null | undefined
 >;
+export interface ValidationDataTypes {
+    categorizer: PerseusCategorizerValidationData;
+    // "cs-program": PerseusCSProgramValidationData;
+    // definition: PerseusDefinitionValidationData;
+    // dropdown: PerseusDropdownRubric;
+    // explanation: PerseusExplanationValidationData;
+    // expression: PerseusExpressionValidationData;
+    // grapher: PerseusGrapherValidationData;
+    // "graded-group-set": PerseusGradedGroupSetValidationData;
+    // "graded-group": PerseusGradedGroupValidationData;
+    group: PerseusGroupValidationData;
+    // iframe: PerseusIFrameValidationData;
+    // image: PerseusImageValidationData;
+    // "input-number": PerseusInputNumberValidationData;
+    // interaction: PerseusInteractionValidationData;
+    // "interactive-graph": PerseusInteractiveGraphValidationData;
+    // "label-image": PerseusLabelImageValidationData;
+    // matcher: PerseusMatcherValidationData;
+    // matrix: PerseusMatrixValidationData;
+    // measurer: PerseusMeasurerValidationData;
+    // "molecule-renderer": PerseusMoleculeRendererValidationData;
+    // "number-line": PerseusNumberLineValidationData;
+    // "numeric-input": PerseusNumericInputValidationData;
+    // orderer: PerseusOrdererValidationData;
+    // "passage-ref-target": PerseusRefTargetValidationData;
+    // "passage-ref": PerseusPassageRefValidationData;
+    // passage: PerseusPassageValidationData;
+    // "phet-simulation": PerseusPhetSimulationValidationData;
+    // "python-program": PerseusPythonProgramValidationData;
+    plotter: PerseusPlotterValidationData;
+    // radio: PerseusRadioValidationData;
+    // sorter: PerseusSorterValidationData;
+    // table: PerseusTableValidationData;
+    // video: PerseusVideoValidationData;
+
+    // Deprecated widgets
+    // sequence: PerseusAutoCorrectValidationData;
+}
+
+/**
+ * A map of validation data, keyed by `widgetId`. This data is used to check if
+ * a question is answerable. This data represents the minimal intersection of
+ * data that's available in the client (widget options) and server (scoring
+ * data) and is represented by a group of types known as "validation data".
+ *
+ * NOTE:  The value in this map is intentionally a subset of WidgetOptions<T>.
+ * By using the same shape (minus any unneeded data), we are able to pass a
+ * `PerseusWidgetsMap` or ` into any function that accepts a
+ * `ValidationDataMap` without any mutation of data.
+ */
+export type ValidationDataMap = {
+    [Property in keyof ValidationDataTypes as `${Property} ${number}`]: {
+        type: Property;
+        static?: boolean;
+        options: ValidationDataTypes[Property];
+    };
+};
+
+/**
+ * A union type of all the different widget validation data types that exist.
+ */
+export type ValidationData = ValidationDataTypes[keyof ValidationDataTypes];
diff --git a/packages/perseus/src/widgets.ts b/packages/perseus/src/widgets.ts
@@ -12,6 +12,7 @@ import type {
     WidgetExports,
     WidgetTransform,
     WidgetScorerFunction,
+    WidgetValidatorFunction,
 } from "./types";
 import type * as React from "react";
 
@@ -137,6 +138,12 @@ export const getWidgetExport = (name: string): WidgetExports | null => {
     return widgets[name] ?? null;
 };
 
+export const getWidgetValidator = (
+    name: string,
+): WidgetValidatorFunction | null => {
+    return widgets[name]?.validator ?? null;
+};
+
 export const getWidgetScorer = (name: string): WidgetScorerFunction | null => {
     return widgets[name]?.scorer ?? null;
 };

diff --git a/packages/perseus/src/widgets/categorizer/categorizer.tsx b/packages/perseus/src/widgets/categorizer/categorizer.tsx
@@ -17,6 +17,7 @@ import Util from "../../util";
 import {getPromptJSON as _getPromptJSON} from "../../widget-ai-utils/categorizer/categorizer-ai-utils";
 
 import scoreCategorizer from "./score-categorizer";
+import validateCategorizer from "./validate-categorizer";
 
 import type {PerseusCategorizerWidgetOptions} from "../../perseus-types";
 import type {Widget, WidgetExports, WidgetProps} from "../../types";
@@ -328,4 +329,7 @@ export default {
     // TODO(LEMS-2656): remove TS suppression
     // @ts-expect-error: Type 'UserInput' is not assignable to type 'PerseusCSProgramUserInput'.
     scorer: scoreCategorizer,
+    // TODO(LEMS-2656): remove TS suppression
+    // @ts-expect-error: Type 'UserInput' is not assignable to type 'PerseusCSProgramUserInput'.
+    validator: validateCategorizer,
 } satisfies WidgetExports<typeof Categorizer>;
diff --git a/packages/perseus/src/widgets/dropdown/dropdown.tsx b/packages/perseus/src/widgets/dropdown/dropdown.tsx
@@ -9,6 +9,7 @@ import {ApiOptions} from "../../perseus-api";
 import {getPromptJSON as _getPromptJSON} from "../../widget-ai-utils/dropdown/dropdown-ai-utils";
 
 import scoreDropdown from "./score-dropdown";
+import validateDropdown from "./validate-dropdown";
 
 import type {PerseusDropdownWidgetOptions} from "../../perseus-types";
 import type {Widget, WidgetExports, WidgetProps} from "../../types";
@@ -162,4 +163,7 @@ export default {
     // TODO(LEMS-2656): remove TS suppression
     // @ts-expect-error: Type 'UserInput' is not assignable to type 'PerseusDropdownUserInput'.
     scorer: scoreDropdown,
+    // TODO(LEMS-2656): remove TS suppression
+    // @ts-expect-error: Type 'UserInput' is not assignable to type 'PerseusDropdownUserInput'.
+    validator: validateDropdown,
 } satisfies WidgetExports<typeof Dropdown>;
diff --git a/packages/perseus/src/widgets/expression/expression.tsx b/packages/perseus/src/widgets/expression/expression.tsx
@@ -20,6 +20,7 @@ import {getPromptJSON as _getPromptJSON} from "../../widget-ai-utils/expression/
 
 import getDecimalSeparator from "./get-decimal-separator";
 import scoreExpression from "./score-expression";
+import validateExpression from "./validate-expression";
 
 import type {DependenciesContext} from "../../dependencies";
 import type {PerseusExpressionWidgetOptions} from "../../perseus-types";
@@ -558,6 +559,9 @@ export default {
     // TODO(LEMS-2656): remove TS suppression
     // @ts-expect-error: Type 'UserInput' is not assignable to type 'PerseusExpressionUserInput'.
     scorer: scoreExpression,
+    // TODO(LEMS-2656): remove TS suppression
+    // @ts-expect-error: Type 'UserInput' is not assignable to type 'PerseusExpressionUserInput'.
+    validator: validateExpression,
 
     // TODO(LEMS-2656): remove TS suppression
     // @ts-expect-error: Type 'Rubric' is not assignable to type 'PerseusExpressionRubric'.