[LEMS-2851] Answerless NumericInput #2249

handeyeco · 2025-02-18T22:17:25Z

Summary:

🚨🚨🚨 Will likely be closed in favor of: #2261

NumericInput needed to be adjusted for answerless rendering because it was using answers.answerforms to generate a set of answer forms it could use when generating text for a tooltip that's displayed pre-scoring (see the ticket for more information).

This PR adds PerseusNumericInputWidgetOptions.fullAnswerForms (PerseusNumericInputWidgetOptions.answerForms was already taken although it's deprecated), a new NumericInput widget version, and upgrade functionality to derive fullAnswerForms from answers.answerforms. I am totally willing to rename fullAnswerForms, it's just a placeholder as I work. In case you're confused:

PerseusNumericInputWidgetOptions.answerForms: deprecated but might still exist for old data (though I don't think it's used)
PerseusNumericInputAnswer.answerForms: current way we derive answerForms data
PerseusNumericInputWidgetOptions.fullAnswerForms: what I add in this PR so we can get answerForms without answers

If you're still confused, I'm happy to do a tour.

Issue: LEMS-2851

Test plan:

Go to a NumericInput widget that is known to show the tooltip that starts with "Your answer should be..."
That tooltip should continue to be visible
NumericInput should be renderable, interactive, and answerable.

… transform

… examples

github-actions · 2025-02-18T22:21:04Z

npm Snapshot: Published

Good news!! We've packaged up the latest commit from this PR (d60dfba) and published it to npm. You
can install it using the tag PR2249.

Example:

yarn add @khanacademy/perseus@PR2249

If you are working in Khan Academy's webapp, you can run:

./dev/tools/bump_perseus_version.sh -t PR2249

github-actions · 2025-02-18T22:21:28Z

Size Change: +471 B (+0.03%)

Total Size: 1.48 MB

Filename	Size	Change
`packages/perseus-core/dist/es/index.js`	47.2 kB	+811 B (+1.75%)
`packages/perseus/dist/es/index.js`	377 kB	-340 B (-0.09%)

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	39 kB
`packages/keypad-context/dist/es/index.js`	760 B
`packages/kmath/dist/es/index.js`	86.9 kB
`packages/math-input/dist/es/index.js`	77.7 kB
`packages/math-input/dist/es/strings.js`	1.79 kB
`packages/perseus-editor/dist/es/index.js`	688 kB
`packages/perseus-linter/dist/es/index.js`	22.2 kB
`packages/perseus-score/dist/es/index.js`	115 kB
`packages/perseus/dist/es/strings.js`	6.06 kB
`packages/pure-markdown/dist/es/index.js`	3.66 kB
`packages/simple-markdown/dist/es/index.js`	12.5 kB

_{compressed-size-action}

handeyeco · 2025-02-18T22:22:56Z

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

@@ -1205,6 +1205,7 @@ export type PerseusNumericInputWidgetOptions = {
    // Used by examples, maybe not used and should be removed in the future
    // see TODO in numeric-input
    answerForms?: ReadonlyArray<PerseusNumericInputAnswerForm>;
+    fullAnswerForms?: ReadonlyArray<PerseusNumericInputAnswerForm>;


I'm expecting that this will change (or something will need to change). Here's what I think happened:

Originally NumericInput used PerseusNumericInputWidgetOptions.answerForms

Then NumericInput was updated to use PerseusNumericInputAnswer.answerForms and PerseusNumericInputWidgetOptions.answerForms was deprecated (see: "maybe not used and should be removed in the future")

Now PerseusNumericInputWidgetOptions.answers is being removed and I think we need something like PerseusNumericInputWidgetOptions.answerForms again but PerseusNumericInputWidgetOptions.answerForms might exist on old data so I called the new thing fullAnswerForms pending more research.

handeyeco · 2025-02-18T22:24:03Z

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

+    };
+}
+
+export const parseNumericInputWidget: Parser<NumericInputWidget> =


Upgrader in Ben's parser

handeyeco · 2025-02-18T22:24:58Z

packages/perseus-core/src/widgets/numeric-input/numeric-input-upgrade.ts

+
+export const currentVersion = {major: 1, minor: 0};
+
+export const widgetOptionsUpgrades = {


Upgrader using the widgetOptionsUpgrades API (which will likely be replaced by Ben's parser)

handeyeco · 2025-02-18T22:26:51Z

packages/perseus/src/widgets/numeric-input/numeric-input.class.tsx

-    return rendererProps;
+    return {
+        ...rendererProps,
+        answerForms: fullAnswerForms,


Like I said, we might not keep it named fullAnswerForms so for now I'm just renaming it to answerForms so I don't have to update the widget code which I'd be fine doing once we have an established name.

handeyeco · 2025-02-18T22:27:22Z

packages/perseus/src/widgets/numeric-input/numeric-input.test.ts

-        });
-    });
-
-    it("only uses answer forms from correct answers", () => {


This got moved to tests for deriveAnswerForms

handeyeco · 2025-02-18T22:29:21Z

packages/perseus/src/widgets/numeric-input/utils.ts

@@ -42,8 +46,16 @@ export const generateExamples = (
    // Generate a list of the unique answer forms.
    const uniqueForms = getUniqueAnswerForms(answerForms);

+    // Sort them by the order they appear in the `formExamples` list.
+    const formExampleKeys = Object.keys(NumericExampleStrings);
+    const sortedForms = uniqueForms.sort((a, b) => {


So originally we were sorting answerForms when deriving the data, but:

I think that's a UI concern, not a data concern

I didn't want to have to move NumericExampleStrings into perseus-core

So I moved the sorting logic and added a test.

handeyeco · 2025-02-18T22:29:57Z

packages/perseus/src/widgets/numeric-input/utils.ts

- * answer forms. This is useful for ensuring that we don't show duplicate examples
- * to the user.
- */
-const getUniqueAnswerForms = function (


These were moved to perseus-core because I needed them for deriving answer forms.

handeyeco · 2025-02-18T23:08:20Z

packages/perseus/src/widgets/numeric-input/numeric-input.test.ts

+        expect(score).toHaveBeenAnsweredCorrectly();
+    });
+
+    it("is interactive with answerless widget options", async () => {


Here's the important test. We:

Render the widget with the stripped data

Assert that UI that previously required answers still renders

Answer and score the widget

benchristel · 2025-02-19T00:08:55Z

packages/perseus-core/src/widgets/numeric-input/util.ts

+        }
+        // Otherwise, add it to the set and return true.
+        foundForms.add(form.name);
+        return true;


Is it correct to not take simplify into account when de-duplicating answer forms? E.g. if we have

const answerForms = [ {name: "proper", simplify: "required"}, {name: "proper", simplify: "optional"}, ];

Only {name: "proper", simplify: "required"} will be present in the output. Whether simplify is "required" or "optional" in the output depends on the order in which the answers were originally listed, which is awkward, since I don't think the order is otherwise important.

I noticed there's a very similar function, unionAnswerForms, in numeric-input.tsx. I think yours is better (better-typed, easier to read, possibly faster) but the two functions should probably be combined if they represent the same concept.

benchristel · 2025-02-19T00:17:48Z

packages/perseus-core/src/widgets/numeric-input/util.ts

+        // so that we can generate the examples for the widget.
+        (options.answers || [])
+            .filter((answer) => answer.status === "correct")
+            .map((answer) => {


If we used flatMap here I don't think we'd need the call to flat() in unionAnswerForms. unionAnswerForms would then be a trivial wrapper around getUniqueAnswerForms, so it could be removed.

Suggested change

.map((answer) => {

.flatMap((answer) => {

handeyeco · 2025-02-20T16:24:02Z

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

+
+import {parseNumericInputWidget} from "./numeric-input-widget";
+
+describe("parseExpressionWidget", () => {


Copy pasta mistake

## Summary: Alternative to #2249 Instead of doing a widget upgrade (the old PR), just split `answers` more selectively so that the data we need for rendering pre-scoring is still available. Issue: LEMS-2851 ## Test plan: - Go to a NumericInput widget that is known to show the tooltip that starts with "Your answer should be..." - That tooltip should continue to be visible - NumericInput should be renderable, interactive, and answerable. Author: handeyeco Reviewers: benchristel, Myranae, handeyeco, jeremywiebe Required Reviewers: Approved By: benchristel Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x), ✅ Cypress (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x) Pull Request URL: #2261

handeyeco added 6 commits February 18, 2025 14:54

[LEMS-2851/answerless-numeric-input] add some additional tests for NI…

c54d7cb

… transform

[LEMS-2851/answerless-numeric-input] split out deriveAnswerForms

76ffb51

[LEMS-2851/answerless-numeric-input] sort answerForms when generating…

3c828e9

… examples

[LEMS-2851/answerless-numeric-input] move utils out of perseus

2d25b9d

[LEMS-2851/answerless-numeric-input] add widgetOptionsUpgrade

8e42d2b

[LEMS-2851/answerless-numeric-input] add upgrade to parser

df8880f

handeyeco self-assigned this Feb 18, 2025

handeyeco changed the title ~~[LEMS-2851/answerless-numeric-input] add some additional tests for NI transform~~ [LEMS-2851] Answerless NumericInput Feb 18, 2025

handeyeco commented Feb 18, 2025

View reviewed changes

[LEMS-2851/answerless-numeric-input] remove commented code

6c7fc69

handeyeco requested review from jeremywiebe, benchristel, SonicScrewdriver, Myranae and a team February 18, 2025 22:39

[LEMS-2851/answerless-numeric-input] add full interactive test

d60dfba

handeyeco commented Feb 18, 2025

View reviewed changes

benchristel reviewed Feb 19, 2025

View reviewed changes

handeyeco commented Feb 20, 2025

View reviewed changes

handeyeco mentioned this pull request Feb 20, 2025

[LEMS-2851] Answerless NumericInput (Take 2) #2261

Merged

handeyeco closed this Feb 20, 2025

handeyeco deleted the LEMS-2851/answerless-numeric-input branch February 20, 2025 22:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LEMS-2851] Answerless NumericInput #2249

[LEMS-2851] Answerless NumericInput #2249

handeyeco commented Feb 18, 2025 •

edited

Loading

github-actions bot commented Feb 18, 2025 •

edited

Loading

github-actions bot commented Feb 18, 2025 •

edited

Loading

handeyeco Feb 18, 2025

handeyeco Feb 18, 2025

handeyeco Feb 18, 2025

handeyeco Feb 18, 2025

handeyeco Feb 18, 2025

handeyeco Feb 18, 2025

handeyeco Feb 18, 2025

handeyeco Feb 18, 2025

benchristel Feb 19, 2025

benchristel Feb 19, 2025

benchristel Feb 19, 2025

handeyeco Feb 20, 2025


		export const currentVersion = {major: 1, minor: 0};

		export const widgetOptionsUpgrades = {


		import {parseNumericInputWidget} from "./numeric-input-widget";

		describe("parseExpressionWidget", () => {

[LEMS-2851] Answerless NumericInput #2249

[LEMS-2851] Answerless NumericInput #2249

Conversation

handeyeco commented Feb 18, 2025 • edited Loading

Summary:

Test plan:

github-actions bot commented Feb 18, 2025 • edited Loading

npm Snapshot: Published

github-actions bot commented Feb 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

handeyeco commented Feb 18, 2025 •

edited

Loading

github-actions bot commented Feb 18, 2025 •

edited

Loading

github-actions bot commented Feb 18, 2025 •

edited

Loading