Udpate Radio's userInput and rubric to be as focused as possible #1844

Myranae · 2024-11-12T17:15:59Z

Summary:

There were a number of properties on Radio's rubric and userInput objects that did not need to be there. This work refines the objects to only be what is required from the user and what is required for scoring.

Issue: LEMS-2541

Test plan:

Confirm all checks pass
Confirm widget still works as expected

Used the calculation that is used elsewhere within the validator to determine numCorrect

github-actions · 2024-11-12T17:18:31Z

npm Snapshot: Published

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

Example:

yarn add @khanacademy/perseus@PR1844

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

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

github-actions · 2024-11-12T17:18:47Z

Size Change: -153 B (-0.02%)

Total Size: 866 kB

Filename	Size	Change
`packages/perseus/dist/es/index.js`	416 kB	-153 B (-0.04%)

ℹ️ View Unchanged

Filename	Size
`packages/kas/dist/es/index.js`	38.8 kB
`packages/keypad-context/dist/es/index.js`	760 B
`packages/kmath/dist/es/index.js`	4.27 kB
`packages/math-input/dist/es/index.js`	77.8 kB
`packages/math-input/dist/es/strings.js`	1.79 kB
`packages/perseus-core/dist/es/index.js`	1.48 kB
`packages/perseus-editor/dist/es/index.js`	283 kB
`packages/perseus-linter/dist/es/index.js`	22.2 kB
`packages/perseus/dist/es/strings.js`	3.54 kB
`packages/pure-markdown/dist/es/index.js`	3.66 kB
`packages/simple-markdown/dist/es/index.js`	12.4 kB

_{compressed-size-action}

khan-actions-bot · 2024-11-12T17:30:54Z

Gerald

Required Reviewers

@Khan/perseus for changes to .changeset/bright-countries-peel.md, packages/perseus/src/validation.types.ts, packages/perseus/src/__tests__/widgets.test.ts, packages/perseus/src/multi-items/__tests__/multi-renderer.test.tsx, packages/perseus/src/widget-ai-utils/group/group.test.ts, packages/perseus/src/widget-ai-utils/group/prompt-utils.test.ts, packages/perseus/src/widget-ai-utils/radio/prompt-utils.test.ts, packages/perseus/src/widget-ai-utils/radio/prompt-utils.ts, packages/perseus/src/widgets/group/group.test.tsx, packages/perseus/src/widgets/radio/radio-component.tsx, packages/perseus/src/widgets/radio/radio-validator.test.ts, packages/perseus/src/widgets/radio/radio-validator.ts

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

handeyeco

Thanks Tamara, I like this. A good step forward both for our work and Perseus as a whole.

handeyeco · 2024-11-12T20:46:29Z

packages/perseus/src/widgets/radio/radio-validator.ts

+    const numCorrect: number = _.reduce(
+        rubric.choices,
+        function (memo, choice) {
+            return choice.correct ? memo + 1 : memo;
+        },
+        0,
+    );


When it's an option, I'd like to move away from underscore and use native JS; reduce is native now.

Suggested change

const numCorrect: number = _.reduce(

rubric.choices,

function (memo, choice) {

return choice.correct ? memo + 1 : memo;

},

0,

);

const numCorrect: number = rubric.choices.reduce(

(acc, curr) => {

return curr.correct ? acc + 1 : acc;

},

0,

);

I also try to get rid of underscore stuff. Sometimes my brain refuses to realize I can update something I copy and pasted from another part of the codebase 😂 Thanks! Updated~

handeyeco · 2024-11-12T20:48:36Z

packages/perseus/src/widgets/radio/radio-validator.ts

+        0,
+    );
+
+    if (numCorrect && numCorrect > 1 && numSelected !== numCorrect) {


Suggested change

if (numCorrect && numCorrect > 1 && numSelected !== numCorrect) {

if (numCorrect > 1 && numSelected !== numCorrect) {

I know you're just moving existing code, but numCorrect is a number and the only falsy number is 0 which is not greater than 1.

handeyeco · 2024-11-12T20:56:34Z

packages/perseus/src/widgets/radio/radio-validator.ts

+    let noneOfTheAboveSelected = false;
+    for (let i = 0; i < rubric.choices.length; i++) {
+        if (
+            rubric.choices[i].isNoneOfTheAbove &&
+            userInput.choicesSelected[i]
+        ) {
+            noneOfTheAboveSelected = true;
+            break;
+        }
+    }


Your code is clearer, but just because I like the new methods here is an alternative:

Suggested change

let noneOfTheAboveSelected = false;

for (let i = 0; i < rubric.choices.length; i++) {

if (

rubric.choices[i].isNoneOfTheAbove &&

userInput.choicesSelected[i]

) {

noneOfTheAboveSelected = true;

break;

}

}

let noneOfTheAboveSelected = rubric.choices.some((e, i) => (

e.isNoneOfTheAbove && userInput.choicesSelected[i]

)

Oh, I love this. Definitely updated to your alternative, though I tried to make the arguments a bit more descriptive :)

- Remove underscore and replace with native js - remove unneeded part of conditional - Simplify with array method

@Myranae

This PR was opened by the [Changesets release](https://github.com/changesets/action) GitHub action. When you're ready to do a release, you can merge this and the packages will be published to npm automatically. If you're not ready to do a release yet, that's fine, whenever you add more changesets to main, this PR will be updated. # Releases ## @khanacademy/perseus@41.3.0 ### Minor Changes - [#1848](#1848) [`55371ded7`](55371de) Thanks [@Myranae](https://github.com/Myranae)! - Rename all validator functions and related code to reference scoring instead - [#1754](#1754) [`77f1bf98f`](77f1bf9) Thanks [@MikeKlemarewski](https://github.com/MikeKlemarewski)! - Adds the getPromptJSON utility method to the renderer and every widget. It can be used to get a JSON representation of the exercise to pass to an LLM. ### Patch Changes - [#1844](#1844) [`092c81f6c`](092c81f) Thanks [@Myranae](https://github.com/Myranae)! - Update Radio's userInput and rubric objects and types - [#1837](#1837) [`80f0480e6`](80f0480) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Type widget exports using 'satisfies' keyword instead of 'as' - [#1785](#1785) [`32b27322c`](32b2732) Thanks [@benchristel](https://github.com/benchristel)! - Internal: add a parsing layer to typecheck PerseusItem data at runtime (ADR #773). The parsing code is not used yet. - [#1838](#1838) [`08fa66103`](08fa661) Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Introduce a PerseusArticle type - [#1825](#1825) [`3dd1fa5ce`](3dd1fa5) Thanks [@catandthemachines](https://github.com/catandthemachines)! - Adding stronger typing to expression editor issues. - [#1847](#1847) [`dcb4b27b3`](dcb4b27) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Labels] Allow the usage of `{$}` to denote actual dollar signs within locked label TeX - [#1850](#1850) [`61dbd4a2c`](61dbd4a) Thanks [@MikeKlemarewski](https://github.com/MikeKlemarewski)! - Export WidgetPromptJSON type so it is accessible - [#1842](#1842) [`cf2ea471e`](cf2ea47) Thanks [@MikeKlemarewski](https://github.com/MikeKlemarewski)! - Fix function signature of getUserInputFromProps for radio widget ## @khanacademy/perseus-editor@14.11.1 ### Patch Changes - [#1825](#1825) [`3dd1fa5ce`](3dd1fa5) Thanks [@catandthemachines](https://github.com/catandthemachines)! - Adding stronger typing to expression editor issues. - [#1847](#1847) [`dcb4b27b3`](dcb4b27) Thanks [@nishasy](https://github.com/nishasy)! - [Locked Labels] Allow the usage of `{$}` to denote actual dollar signs within locked label TeX - Updated dependencies \[[`092c81f6c`](092c81f), [`80f0480e6`](80f0480), [`32b27322c`](32b2732), [`08fa66103`](08fa661), [`55371ded7`](55371de), [`3dd1fa5ce`](3dd1fa5), [`dcb4b27b3`](dcb4b27), [`61dbd4a2c`](61dbd4a), [`77f1bf98f`](77f1bf9), [`cf2ea471e`](cf2ea47)]: - @khanacademy/perseus@41.3.0 Author: khan-actions-bot Reviewers: Myranae Required Reviewers: Approved By: Myranae Checks: ⏭️ Publish npm snapshot, ✅ Cypress (ubuntu-latest, 20.x), ✅ Check builds for changes in size (ubuntu-latest, 20.x), ✅ gerald, ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Check for .changeset entries for all changed files (ubuntu-latest, 20.x) Pull Request URL: #1840

Myranae added 6 commits November 7, 2024 09:25

Remove numCorrect from rubric and userinput

29d8c3f

Used the calculation that is used elsewhere within the validator to determine numCorrect

Remove notaIndex from userInput

009bd41

Remove notaSelected from UserInput

89b59e4

Merge branch 'main' into tb/LEMS-2541/clean-up-radio-user-input

f3c7719

Remove new notaSelected references

7aae2f1

Merge branch 'main' into tb/LEMS-2541/clean-up-radio-user-input

992e9bd

Myranae self-assigned this Nov 12, 2024

Myranae added 2 commits November 12, 2024 11:22

Add changeset

bea6a85

Remove properties no longer on radio's userInput

1f3a831

Myranae marked this pull request as ready for review November 12, 2024 17:30

Myranae requested review from handeyeco and jeremywiebe November 12, 2024 17:30

khan-actions-bot requested a review from a team November 12, 2024 17:30

Add back info that was unrelated

3dfaf4d

handeyeco approved these changes Nov 12, 2024

View reviewed changes

Myranae added 3 commits November 12, 2024 15:23

Update based on PR review feedback

5928467

- Remove underscore and replace with native js - remove unneeded part of conditional - Simplify with array method

Remove unneeded import

166f709

Merge branch 'main' into tb/LEMS-2541/clean-up-radio-user-input

3ada8da

Myranae merged commit 092c81f into main Nov 12, 2024
8 of 9 checks passed

Myranae deleted the tb/LEMS-2541/clean-up-radio-user-input branch November 12, 2024 21:52

khan-actions-bot mentioned this pull request Nov 12, 2024

Version Packages #1840

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Udpate Radio's userInput and rubric to be as focused as possible #1844

Udpate Radio's userInput and rubric to be as focused as possible #1844

Myranae commented Nov 12, 2024

github-actions bot commented Nov 12, 2024 •

edited

Loading

github-actions bot commented Nov 12, 2024 •

edited

Loading

khan-actions-bot commented Nov 12, 2024

handeyeco left a comment

handeyeco Nov 12, 2024

Myranae Nov 12, 2024

handeyeco Nov 12, 2024

Myranae Nov 12, 2024

handeyeco Nov 12, 2024

Myranae Nov 12, 2024

	if (numCorrect && numCorrect > 1 && numSelected !== numCorrect) {
	if (numCorrect > 1 && numSelected !== numCorrect) {

Udpate Radio's userInput and rubric to be as focused as possible #1844

Udpate Radio's userInput and rubric to be as focused as possible #1844

Conversation

Myranae commented Nov 12, 2024

Summary:

Test plan:

github-actions bot commented Nov 12, 2024 • edited Loading

npm Snapshot: Published

github-actions bot commented Nov 12, 2024 • edited Loading

khan-actions-bot commented Nov 12, 2024

Gerald

handeyeco left a comment

Choose a reason for hiding this comment

handeyeco Nov 12, 2024

Choose a reason for hiding this comment

Myranae Nov 12, 2024

Choose a reason for hiding this comment

handeyeco Nov 12, 2024

Choose a reason for hiding this comment

Myranae Nov 12, 2024

Choose a reason for hiding this comment

handeyeco Nov 12, 2024

Choose a reason for hiding this comment

Myranae Nov 12, 2024

Choose a reason for hiding this comment

github-actions bot commented Nov 12, 2024 •

edited

Loading

github-actions bot commented Nov 12, 2024 •

edited

Loading