docs(changeset): Fixed bugs related to numCorrect not updating properly for the Radio widget #2630

SonicScrewdriver · 2025-06-20T22:25:43Z

Summary:

We had 2 bugs reported related to the numCorrect value not correctly updating for the Radio Widget.

This PR fixes two of these bugs by ensuring that we're recalculating the numCorrect value when we delete choices, or switch in and out of multiple selection mode.

Unfortunately, I have not been able to recreate the third bug in the ticket.

Issue: LEMS-3230

Test plan:

New tests pass

… updating properly for the Radio widget

github-actions · 2025-06-20T22:27:02Z

Size Change: -55 B (-0.01%)

Total Size: 472 kB

Filename	Size	Change
`packages/perseus-core/dist/es/index.js`	20.3 kB	-11 B (-0.05%)
`packages/perseus-editor/dist/es/index.js`	91.1 kB	-44 B (-0.05%)

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	20.7 kB
`packages/keypad-context/dist/es/index.js`	1 kB
`packages/kmath/dist/es/index.js`	5.98 kB
`packages/math-input/dist/es/index.js`	98.6 kB
`packages/math-input/dist/es/strings.js`	1.61 kB
`packages/perseus-linter/dist/es/index.js`	7.14 kB
`packages/perseus-score/dist/es/index.js`	9.23 kB
`packages/perseus-utils/dist/es/index.js`	403 B
`packages/perseus/dist/es/index.js`	200 kB
`packages/perseus/dist/es/strings.js`	7.56 kB
`packages/pure-markdown/dist/es/index.js`	1.22 kB
`packages/simple-markdown/dist/es/index.js`	6.72 kB

_{compressed-size-action}

github-actions · 2025-06-20T22:29:20Z

npm Snapshot: Published

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

Example:

pnpm add @khanacademy/perseus@PR2630

If you are working in Khan Academy's frontend, you can run the below command.

./tools/bump_perseus_version.js -t PR2630

If you are working in Khan Academy's webapp, you can run the below command.

./dev/tools/bump_perseus_version.js -s n -t PR2630

mark-fitzgerald · 2025-06-24T16:18:44Z

packages/perseus-editor/src/widgets/radio/editor.tsx

+        let choices = this.props.choices;
+        if (!multipleSelect) {
+            const numCorrect = _.reduce(
+                this.props.choices,


Should this be the local choices object defined on line 123?

mark-fitzgerald · 2025-06-24T16:20:17Z

packages/perseus-editor/src/widgets/radio/editor.tsx

+        // we need to deselect all options
+        let choices = this.props.choices;
+        if (!multipleSelect) {
+            const numCorrect = _.reduce(


Would this be a good time to remove underscore, or would that be a big distraction from the purpose of this PR?

Let's do it! I'm down for any excuse to remove it

mark-fitzgerald · 2025-06-24T16:25:12Z

packages/perseus-editor/src/widgets/radio/editor.tsx

-            this.props.onChange({
-                multipleSelect: allowMultiple,
-            });
+        const multipleSelect = allowMultiple.multipleSelect;


Do we have a naming convention for boolean values like this? For instance, isMultipleSelect?

Tbh I don't know! It just bugged me that we were extracting multipleSelect out of allowMultiple and then calling it allowMultiple. 😅 I'm happy to make it isMultipleSelect though!

mark-fitzgerald · 2025-06-24T22:11:30Z

packages/perseus-editor/src/widgets/__tests__/radio-editor.test.tsx

+        // Check that multipleSelect is updated and numCorrect is calculated
+        expect(onChangeMock).toHaveBeenCalledWith(
+            expect.objectContaining({
+                numCorrect: expect.any(Number),


How does this show that numCorrect has been calculated?

It doesn't! Apparently my tired Friday brain didn't quite finish up this PR. I've fixed this up quite a bit!

mark-fitzgerald · 2025-06-24T22:13:28Z

packages/perseus-editor/src/widgets/__tests__/radio-editor.test.tsx

+    it("updates numCorrect when deleting an option", async () => {
+        const onChangeMock = jest.fn();
+
+        function getCorrectChoice(): PerseusRadioChoice {


Can this function and its "incorrect" counterpart be hoisted to the describe parent instead of rebuilt for each test?

… a test, and cleaning up expects

SonicScrewdriver · 2025-06-25T18:59:06Z

packages/perseus-editor/src/widgets/radio/editor.tsx

@@ -116,39 +116,44 @@ class RadioEditor extends React.Component<RadioEditorProps> {
    };

    onMultipleSelectChange: (arg1: any) => any = (allowMultiple) => {
-        allowMultiple = allowMultiple.multipleSelect;
-
-        const numCorrect = _.reduce(


No idea why were were doing this manually every time. We already have a nifty little method to handle this (deriveNumCorrect)!

SonicScrewdriver · 2025-06-25T19:07:38Z

packages/perseus-editor/src/widgets/radio/editor.tsx

+        const countChoices = count.countChoices;
+        this.props.onChange({
+            countChoices,
+        });
    };


This was silly naming, imo.

SonicScrewdriver · 2025-06-25T19:12:43Z

packages/perseus-core/src/widgets/radio/derive-num-correct.ts


-    return numCorrect ?? choices.filter((c) => c.correct).length;
+    return choices.filter((c) => c.correct).length;
 }


This could be a spicy move, but I really don't understand why we were ever returning numCorrect in the first place, as everything I saw indicates that we always want to recalculate this value.

This is only used in the radio editor.tsx file with undefined for numCorrect, and in our parser for converting a Radio widget from v1 to a v2 as v1 did not have this option.

…tions anymore.

… updating properly for the Radio widget, and cleaning up deriveNumCorrect.

anakaren-rojas · 2025-06-26T22:08:56Z

packages/perseus-core/src/widgets/radio/derive-num-correct.ts

-
-    return numCorrect ?? choices.filter((c) => c.correct).length;
+export function deriveNumCorrect(
+    choices: PerseusRadioWidgetOptions["choices"],


should this be destructured?

Suggested change

choices: PerseusRadioWidgetOptions["choices"],

{choices} : PerseusRadioWidgetOptions,

Oooo it could, but I think we've generally used the first format 🤔.

anakaren-rojas

🌶️ move checks out

SonicScrewdriver added 2 commits June 20, 2025 15:23

[um-correction] Fixed Case 1 and 2

247ced3

[um-correction] docs(changeset): Fixed bugs related to numCorrect not…

67733f4

… updating properly for the Radio widget

SonicScrewdriver self-assigned this Jun 20, 2025

SonicScrewdriver added the olc-5.0.0171b label Jun 20, 2025

SonicScrewdriver marked this pull request as ready for review June 20, 2025 22:27

SonicScrewdriver requested review from ivyolamit, mark-fitzgerald, anakaren-rojas and nishasy as code owners June 20, 2025 22:27

mark-fitzgerald reviewed Jun 24, 2025

View reviewed changes

SonicScrewdriver added 3 commits June 25, 2025 11:01

[um-correction] fixing several tired mistakes!

2058438

[um-correction] missed a spot, fixing test name to be clearer, adding…

2543bde

… a test, and cleaning up expects

[um-correction] fix a word in test name

fae9048

SonicScrewdriver commented Jun 25, 2025

View reviewed changes

[um-correction] spicier change

ca0c58a

SonicScrewdriver commented Jun 25, 2025

View reviewed changes

[um-correction] clean up tests

22df5d5

SonicScrewdriver commented Jun 25, 2025

View reviewed changes

SonicScrewdriver added 5 commits June 25, 2025 13:04

[um-correction] comment cleanup

cfe97e3

[um-correction] Missed an undefined

53be711

[um-correction] There's really no reason to pass in all the widget op…

339f381

…tions anymore.

[um-correction] docs(changeset): Fixed bugs related to numCorrect not…

534a072

… updating properly for the Radio widget, and cleaning up deriveNumCorrect.

[um-correction] remove old changeset

2cccb7c

anakaren-rojas reviewed Jun 26, 2025

View reviewed changes

anakaren-rojas approved these changes Jun 26, 2025

View reviewed changes

	choices: PerseusRadioWidgetOptions["choices"],
	{choices} : PerseusRadioWidgetOptions,

docs(changeset): Fixed bugs related to numCorrect not updating properly for the Radio widget #2630

Are you sure you want to change the base?

docs(changeset): Fixed bugs related to numCorrect not updating properly for the Radio widget #2630

Conversation

SonicScrewdriver commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary:

Test plan:

Uh oh!

github-actions bot commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

npm Snapshot: Published

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SonicScrewdriver Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anakaren-rojas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SonicScrewdriver commented Jun 20, 2025 •

edited

Loading

github-actions bot commented Jun 20, 2025 •

edited

Loading

github-actions bot commented Jun 20, 2025 •

edited

Loading

SonicScrewdriver Jun 25, 2025 •

edited

Loading