Handle all legacy English content in the Perseus parser #2073

benchristel · 2025-01-06T19:07:14Z

This PR adds regression tests for English-language exercises that were
previously not accepted by the Perseus JSON parser. It also updates the parser
to accept those exercises and convert them to the latest format.

As of this commit, all English exercises can now be parsed!

Issue: LEMS-2582

Test plan:

yarn test

benchristel · 2025-01-06T19:08:24Z

Note to reviewers: this PR will be easiest to review one commit at a time.

benchristel · 2025-01-06T19:09:15Z

packages/perseus/src/perseus-types.ts

@@ -1215,7 +1218,7 @@ export type PerseusNumberLineWidgetOptions = {
    // The correct relative value. default: "eq". options: "eq", "lt", "gt", "le", "ge"
    correctRel: string | null | undefined;
    // This is the correct answer. The answer is validated (as right or wrong) by using only the end position of the point and the relation (=, &lt;, &gt;, ≤, ≥).
-    correctX: number;
+    correctX: number | null;


The measurer widget component handles null values.

<rant target='nobody-in-particular'>
I understand the difference between undefined and null, but so often we use these two values in the same way. IMHO it's a paper cut I wish we could remove eventually.
</rant>

github-actions · 2025-01-06T19:11:05Z

npm Snapshot: Published

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

Example:

yarn add @khanacademy/perseus@PR2073

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

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

github-actions · 2025-01-06T19:11:15Z

Size Change: +175 B (+0.01%)

Total Size: 1.28 MB

Filename	Size	Change
`packages/perseus/dist/es/index.js`	419 kB	+175 B (+0.04%)

ℹ️ 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`	4.27 kB
`packages/math-input/dist/es/index.js`	78 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`	688 kB
`packages/perseus-linter/dist/es/index.js`	22.2 kB
`packages/perseus/dist/es/strings.js`	5.03 kB
`packages/pure-markdown/dist/es/index.js`	3.67 kB
`packages/simple-markdown/dist/es/index.js`	12.5 kB

_{compressed-size-action}

packages/perseus/src/widgets/matrix/score-matrix.test.ts

…parsing

… versionedWidgetOptions parser

…andle all English-language exercises

to ensure those widgets are always marked incorrect (preserving the current behavior). Conveniently, JSON.stringify(NaN) === "null", so this also preserves the identity serialize(parse(X)) === X.

jeremywiebe · 2025-01-07T00:12:15Z

packages/perseus/src/perseus-types.ts

@@ -1131,6 +1131,7 @@ export type PerseusMeasurerWidgetOptions = {
    rulerLength: number;
    // Containing area [width, height]
    box: [number, number];
+    // TODO(benchristel): static is not used. Remove it?


I suspect we can. These types were originally generated by iterating over our en content and deriving TypeScript types from them. I suspect static shows up in these widget options due to bugs that existed for some amount of time in our editors but have since been fixed (or not).

jeremywiebe · 2025-01-07T00:28:22Z

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

@@ -57,7 +57,7 @@ export const parseNumericInputWidget: Parser<NumericInputWidget> = parseWidget(
        ),
        labelText: optional(string),
        size: string,
-        coefficient: boolean,
+        coefficient: defaulted(boolean, () => false),


Going through these regression tests has started me thinking. The widget React components often provide default props which fill in these missing values (eg. coefficient is defaulted to false here).

Sorry, I have post-vacation brain, but have we discussed this before?

My thoughts on where to put defaults are described in the parse-perseus-json/README.md. I think we want them in both the parser and the component's default props, because those defaults serve different purposes.

jeremywiebe · 2025-01-07T00:30:31Z

packages/perseus/src/perseus-types.ts

@@ -1215,7 +1218,7 @@ export type PerseusNumberLineWidgetOptions = {
    // The correct relative value. default: "eq". options: "eq", "lt", "gt", "le", "ge"
    correctRel: string | null | undefined;
    // This is the correct answer. The answer is validated (as right or wrong) by using only the end position of the point and the relation (=, &lt;, &gt;, ≤, ≥).
-    correctX: number;
+    correctX: number | null;


<rant target='nobody-in-particular'>
I understand the difference between undefined and null, but so often we use these two values in the same way. IMHO it's a paper cut I wish we could remove eventually.
</rant>

jeremywiebe · 2025-01-07T00:31:37Z

packages/perseus/src/perseus-types.ts

@@ -333,7 +333,7 @@ export type RefTargetWidget = WidgetOptions<'passage-ref-target', PerseusPassage
 // prettier-ignore
 export type VideoWidget = WidgetOptions<'video', PerseusVideoWidgetOptions>;
 //prettier-ignore
-export type AutoCorrectWidget = WidgetOptions<'deprecated-standin', object>;
+export type DeprecatedStandinWidget = WidgetOptions<'deprecated-standin', object>;


Thank-you!! 😁

jeremywiebe · 2025-01-07T00:31:52Z

packages/perseus/src/util/parse-perseus-json/perseus-parsers/widgets-map.ts

@@ -199,7 +199,7 @@ const parseWidgetsMapEntry: (
    }
 };

-const parseDeprecatedWidget: Parser<AutoCorrectWidget> = parseWidget(


This line makes much more sense now. :)

jeremywiebe · 2025-01-07T00:34:28Z

packages/perseus/src/util/parse-perseus-json/perseus-parsers/cs-program-widget.ts

        width: number,
        height: number,
-        static: boolean,
+        static: defaulted(boolean, () => false),


Fun fact: it seems like width and static are never used by the widget. 🤔

jeremywiebe · 2025-01-07T00:47:50Z

packages/perseus/src/util/parse-perseus-json/perseus-parsers/versioned-widget-options.test.ts

+        const parser = versionedWidgetOptions(2, parseOptionsV2)
+            .withMigrationFrom(1, parseOptionsV1, migrateV1ToV2)
+            .withMigrationFrom(0, parseOptionsV0, migrateV0ToV1).parser;


This reads more clearly. Thanks! I like that the major version is mentioned explicitly here.

Looks like I forgot to change the base branch... this change belongs to another PR. Sorry about that!

jeremywiebe · 2025-01-07T00:51:05Z

packages/perseus/src/util/parse-perseus-json/perseus-parsers/versioned-widget-options.ts

+        private migrateToLatest: (m: MigratableWidgetOptions) => Latest,
+        private parseOtherVersions: Parser<Latest>,


Wait! What? Does this make these two constructor parameters into private class members and assign them automatically? I've never seen this but if that's what it does, that's pretty awesome. :til:

@nishasy

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/[email protected] ### Patch Changes - [#2064](#2064) [`55b4615d3`](55b4615) Thanks [@nishasy](https://github.com/nishasy)! - Remove the locked-figures-aria flag - [#2063](#2063) [`85a5b5e44`](85a5b5e) Thanks [@nishasy](https://github.com/nishasy)! - Remove the interactive-graph-locked-features-labels flag - [#2078](#2078) [`781cc7df6`](781cc7d) Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! - Temporarily fixing pi-based strings for Numeric Input - [#2065](#2065) [`eefcf5c5c`](eefcf5c) Thanks [@nishasy](https://github.com/nishasy)! - Remove the locked-[figureName]-labels flags - [#2068](#2068) [`265a93104`](265a931) Thanks [@benchristel](https://github.com/benchristel)! - Internal: Redesign discriminated union type parser to have a simpler and more intuitive interface. - [#2073](#2073) [`4bf4960d4`](4bf4960) Thanks [@benchristel](https://github.com/benchristel)! - Internal: improve Perseus JSON parsers so they can handle all English-language exercises - [#2080](#2080) [`c9a28b34c`](c9a28b3) Thanks [@mark-fitzgerald](https://github.com/mark-fitzgerald)! - [Dropdown] Bugfix - Text in dropdown not in correct vertical position - [#2073](#2073) [`4bf4960d4`](4bf4960) Thanks [@benchristel](https://github.com/benchristel)! - Internal: improve the error messages produced by the versionedWidgetOptions parser ## @khanacademy/[email protected] ### Patch Changes - [#2064](#2064) [`55b4615d3`](55b4615) Thanks [@nishasy](https://github.com/nishasy)! - Remove the locked-figures-aria flag - [#2063](#2063) [`85a5b5e44`](85a5b5e) Thanks [@nishasy](https://github.com/nishasy)! - Remove the interactive-graph-locked-features-labels flag - [#2065](#2065) [`eefcf5c5c`](eefcf5c) Thanks [@nishasy](https://github.com/nishasy)! - Remove the locked-[figureName]-labels flags - Updated dependencies \[[`55b4615d3`](55b4615), [`85a5b5e44`](85a5b5e), [`781cc7df6`](781cc7d), [`eefcf5c5c`](eefcf5c), [`265a93104`](265a931), [`4bf4960d4`](4bf4960), [`c9a28b34c`](c9a28b3), [`4bf4960d4`](4bf4960)]: - @khanacademy/[email protected]

benchristel self-assigned this Jan 6, 2025

benchristel requested a review from jeremywiebe January 6, 2025 19:07

benchristel commented Jan 6, 2025

View reviewed changes

packages/perseus/src/widgets/matrix/score-matrix.test.ts Outdated Show resolved Hide resolved

Base automatically changed from benc/discriminated-union to main January 7, 2025 00:02

benchristel added 22 commits January 6, 2025 16:04

Default PerseusMeasurerWidgetOptions.static to false

c6307c6

Add TODO comments

3af5044

Default PerseusCategorizerWidgetOptions.randomizeItems to false

fcf7e74

Default PerseusExpressionWidgetOptions.buttonSets

28c4c5f

Default PerseusNumberLineWidgetOptions.static

5e808c1

Add note

943ae5f

Parse deprecated 'simulator' widgets

68c572f

Handle matrices with null values in answer

be75b0c

Default categorizer values to 0

bcf9578

Default grapher coords

3129bed

Fix lint

c796a37

Default dropdown placeholder

f7eec3f

Allow false values for PerseusNumericInputAnswer.simplify

7cb4386

Default PerseusNumericInputWidgetOptions.coefficient to false

7c09e8d

Allow null values for number line correctX

14116fb

Parse deprecated transformer widget

c75489e

Rename AutoCorrectWidget to DeprecatedStandinWidget

82bc0c7

Default CS program static to false

2623860

Default passage footnotes and title

6efd986

Tolerate empty strings in number line labelRange

3803084

Generate snapshots

6f1bfef

Fix lint

262909a

benchristel added 14 commits January 6, 2025 16:04

Default label image static to false

ed4551c

Allow 'exact' as a match value for polygon graphs

58868be

Allow graded group with hint = false

5679ea6

Update snapshots

5867861

Fix lint

46d4bb0

Allow answerForms[i].key to be null in expression widget

1370bce

Refactor

8f6e86b

Use the major version number in the data as the source of truth when …

6e52ffa

…parsing

docs(changeset): Internal: improve the error messages produced by the…

0ed6a8a

… versionedWidgetOptions parser

docs(changeset): Internal: improve Perseus JSON parsers so they can h…

c3ccccd

…andle all English-language exercises

Update snapshots

950a278

Remove outdated snapshot

37449fe

Convert null values in matrix widget answers to NaN

ecef24b

to ensure those widgets are always marked incorrect (preserving the current behavior). Conveniently, JSON.stringify(NaN) === "null", so this also preserves the identity serialize(parse(X)) === X.

Update snapshot

f6b707b

benchristel force-pushed the benc/regression-tests-5 branch from 3c712cb to f6b707b Compare January 7, 2025 00:05

Fix test failure after rebasing

462ca87

jeremywiebe approved these changes Jan 7, 2025

View reviewed changes

Add TODO comments

14f0079

benchristel merged commit 4bf4960 into main Jan 7, 2025
8 checks passed

benchristel deleted the benc/regression-tests-5 branch January 7, 2025 17:22

khan-actions-bot mentioned this pull request Jan 7, 2025

Version Packages #2075

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle all legacy English content in the Perseus parser #2073

Handle all legacy English content in the Perseus parser #2073

benchristel commented Jan 6, 2025 •

edited

Loading

benchristel commented Jan 6, 2025

benchristel Jan 6, 2025

jeremywiebe Jan 7, 2025

github-actions bot commented Jan 6, 2025 •

edited

Loading

github-actions bot commented Jan 6, 2025 •

edited

Loading

jeremywiebe Jan 7, 2025

jeremywiebe Jan 7, 2025

benchristel Jan 7, 2025

jeremywiebe Jan 7, 2025

jeremywiebe Jan 7, 2025

jeremywiebe Jan 7, 2025

jeremywiebe Jan 7, 2025

jeremywiebe Jan 7, 2025

benchristel Jan 7, 2025

jeremywiebe Jan 7, 2025

benchristel Jan 7, 2025

		private migrateToLatest: (m: MigratableWidgetOptions) => Latest,
		private parseOtherVersions: Parser<Latest>,

Handle all legacy English content in the Perseus parser #2073

Handle all legacy English content in the Perseus parser #2073

Conversation

benchristel commented Jan 6, 2025 • edited Loading

Test plan:

benchristel commented Jan 6, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 6, 2025 • edited Loading

npm Snapshot: Published

github-actions bot commented Jan 6, 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

benchristel commented Jan 6, 2025 •

edited

Loading

github-actions bot commented Jan 6, 2025 •

edited

Loading

github-actions bot commented Jan 6, 2025 •

edited

Loading