Skip to content

Commit

Permalink
Enable parsePerseusItem to handle all published Perseus content (#2082)
Browse files Browse the repository at this point in the history
This PR fixes the remaining cases where the parser couldn't handle some data in
our content corpus -- notably, in articles and international content. After this
PR is merged, we will be able to use the parser in Webapp!

Note that running the exhaustive test tool still produces some failures.
However, I suspect the failing content isn't published, because it either
doesn't render (crashes the page) or can't be scored (throws an exception when
you click the "check answer" button). We'll find out when we start logging
parser errors in production whether I'm right about this.

The remaining errors are:

```
(root).question.widgets["grapher N"].options.correct.coords -- expected array of length 2; got []
(root).question.widgets["matcher N"].options -- expected object; got undefined
(root).question.widgets["graded-group N"].options.widgets["numeric-input N"].options.answers[N].answerForms[N] -- expected "integer", "mixed", "improper", "proper", "decimal", "percent", "pi"; got "number"
(root).question.widgets["example-graphie-widget N"] -- expected a valid widget type; got "example-graphie-widget"
(root).question.widgets["image N"]["(widget key)"][1] -- expected a string representing a positive integer; got "0"
(root).question.widgets["explanation N"]["(widget key)"][1] -- expected a string representing a positive integer; got "0"
```

Issue: LEMS-2582

## Test plan:

`yarn test`

Author: benchristel

Reviewers: benchristel, jeremywiebe

Required Reviewers:

Approved By: jeremywiebe

Checks: ✅ Publish npm snapshot (ubuntu-latest, 20.x), ✅ Lint, Typecheck, Format, and Test (ubuntu-latest, 20.x), ✅ Cypress (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), ✅ Publish Storybook to Chromatic (ubuntu-latest, 20.x)

Pull Request URL: #2082
  • Loading branch information
benchristel authored Jan 11, 2025
1 parent 766d335 commit bbf7f3b
Show file tree
Hide file tree
Showing 23 changed files with 3,704 additions and 270 deletions.
6 changes: 6 additions & 0 deletions .changeset/hot-cougars-laugh.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@khanacademy/perseus": minor
"@khanacademy/perseus-core": minor
---

Enable parsePerseusItem to parse all published content, upgrading old formats to the current one.
41 changes: 22 additions & 19 deletions packages/perseus-core/src/data-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -550,11 +550,9 @@ export type GraphRange = [
export type GrapherAnswerTypes =
| {
type: "absolute_value";
coords: [
// The vertex
Coord, // A point along one line of the absolute value "V" lines
Coord,
];
// If `coords` is null, the graph will not be gradable. All answers
// will be scored as invalid.
coords: null | [vertex: Coord, secondPoint: Coord];
}
| {
type: "exponential";
Expand All @@ -563,38 +561,46 @@ export type GrapherAnswerTypes =
asymptote: [Coord, Coord];
// Two points along the exponential curve. One end of the curve
// trends towards the asymptote.
coords: [Coord, Coord];
// If `coords` is null, the graph will not be gradable. All answers
// will be scored as invalid.
coords: null | [Coord, Coord];
}
| {
type: "linear";
// Two points along the straight line
coords: [Coord, Coord];
// If coords is null, the graph will not be gradable. All answers
// will be scored as invalid.
coords: null | [Coord, Coord];
}
| {
type: "logarithm";
// Two points along the asymptote line.
asymptote: [Coord, Coord];
// Two points along the logarithmic curve. One end of the curve
// trends towards the asymptote.
coords: [Coord, Coord];
// If coords is null, the graph will not be gradable. All answers
// will be scored as invalid.
coords: null | [Coord, Coord];
}
| {
type: "quadratic";
coords: [
// The vertex of the parabola
Coord, // A point along the parabola
Coord,
];
// If coords is null, the graph will not be gradable. All answers
// will be scored as invalid.
coords: null | [vertex: Coord, secondPoint: Coord];
}
| {
type: "sinusoid";
// Two points on the same slope in the sinusoid wave line.
coords: [Coord, Coord];
// If coords is null, the graph will not be gradable. All answers
// will be scored as invalid.
coords: null | [Coord, Coord];
}
| {
type: "tangent";
// Two points on the same slope in the tangent wave line.
coords: [Coord, Coord];
// If coords is null, the graph will not be gradable. All answers
// will be scored as invalid.
coords: null | [Coord, Coord];
};

export type PerseusGrapherWidgetOptions = {
Expand Down Expand Up @@ -1615,9 +1621,6 @@ export type PerseusCSProgramWidgetOptions = {
showEditor: boolean;
// Whether to show the execute buttons
showButtons: boolean;
// TODO(benchristel): width is not used. Delete it?
// The width of the widget
width: number;
// The height of the widget
height: number;
// TODO(benchristel): static is not used. Delete it?
Expand All @@ -1643,7 +1646,7 @@ export type PerseusIFrameWidgetOptions = {
// A URL to display OR a CS Program ID
url: string;
// Settings that you add here are available to the program as an object returned by Program.settings()
settings: ReadonlyArray<PerseusCSProgramSetting>;
settings?: ReadonlyArray<PerseusCSProgramSetting>;
// The width of the widget
width: number | string;
// The height of the widget
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ export const parseCSProgramWidget: Parser<CSProgramWidget> = parseWidget(
settings: array(object({name: string, value: string})),
showEditor: boolean,
showButtons: boolean,
width: number,
height: number,
static: defaulted(boolean, () => false),
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
string,
union,
} from "../general-purpose-parsers";
import {defaulted} from "../general-purpose-parsers/defaulted";
import {discriminatedUnionOn} from "../general-purpose-parsers/discriminated-union";

import {parseWidget} from "./widget";
Expand Down Expand Up @@ -42,58 +41,51 @@ export const parseGrapherWidget: Parser<GrapherWidget> = parseWidget(
"absolute_value",
object({
type: constant("absolute_value"),
coords: pairOfPoints,
coords: nullable(pairOfPoints),
}),
)
.withBranch(
"exponential",
object({
type: constant("exponential"),
asymptote: pairOfPoints,
coords: pairOfPoints,
coords: nullable(pairOfPoints),
}),
)
.withBranch(
"linear",
object({
type: constant("linear"),
coords: defaulted(
pairOfPoints,
() =>
[
[-5, 5],
[5, 5],
] as [[number, number], [number, number]],
),
coords: nullable(pairOfPoints),
}),
)
.withBranch(
"logarithm",
object({
type: constant("logarithm"),
asymptote: pairOfPoints,
coords: pairOfPoints,
coords: nullable(pairOfPoints),
}),
)
.withBranch(
"quadratic",
object({
type: constant("quadratic"),
coords: pairOfPoints,
coords: nullable(pairOfPoints),
}),
)
.withBranch(
"sinusoid",
object({
type: constant("sinusoid"),
coords: pairOfPoints,
coords: nullable(pairOfPoints),
}),
)
.withBranch(
"tangent",
object({
type: constant("tangent"),
coords: pairOfPoints,
coords: nullable(pairOfPoints),
}),
).parser,
graph: object({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ export const parseIframeWidget: Parser<IFrameWidget> = parseWidget(
constant("iframe"),
object({
url: string,
settings: array(object({name: string, value: string})),
settings: optional(array(object({name: string, value: string}))),
width: union(number).or(string).parser,
height: union(number).or(string).parser,
allowFullScreen: boolean,
allowFullScreen: defaulted(boolean, () => false),
allowTopNavigation: optional(boolean),
static: defaulted(boolean, () => false),
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ import {
object,
optional,
pair,
pipeParsers,
string,
union,
} from "../general-purpose-parsers";
import {convert} from "../general-purpose-parsers/convert";
import {defaulted} from "../general-purpose-parsers/defaulted";
import {discriminatedUnionOn} from "../general-purpose-parsers/discriminated-union";

Expand All @@ -25,11 +27,12 @@ import type {
const pairOfNumbers = pair(number, number);
const stringOrEmpty = defaulted(string, () => "");

const parseKey = pipeParsers(optional(string)).then(convert(String)).parser;

type FunctionElement = Extract<PerseusInteractionElement, {type: "function"}>;
const parseFunctionType = constant("function");
const parseFunctionElement: Parser<FunctionElement> = object({
type: parseFunctionType,
key: string,
type: constant("function"),
key: parseKey,
options: object({
value: string,
funcName: string,
Expand All @@ -42,10 +45,9 @@ const parseFunctionElement: Parser<FunctionElement> = object({
});

type LabelElement = Extract<PerseusInteractionElement, {type: "label"}>;
const parseLabelType = constant("label");
const parseLabelElement: Parser<LabelElement> = object({
type: parseLabelType,
key: string,
type: constant("label"),
key: parseKey,
options: object({
label: string,
color: string,
Expand All @@ -55,10 +57,9 @@ const parseLabelElement: Parser<LabelElement> = object({
});

type LineElement = Extract<PerseusInteractionElement, {type: "line"}>;
const parseLineType = constant("line");
const parseLineElement: Parser<LineElement> = object({
type: parseLineType,
key: string,
type: constant("line"),
key: parseKey,
options: object({
color: string,
startX: string,
Expand All @@ -75,10 +76,9 @@ type MovableLineElement = Extract<
PerseusInteractionElement,
{type: "movable-line"}
>;
const parseMovableLineType = constant("movable-line");
const parseMovableLineElement: Parser<MovableLineElement> = object({
type: parseMovableLineType,
key: string,
type: constant("movable-line"),
key: parseKey,
options: object({
startX: string,
startY: string,
Expand All @@ -100,10 +100,9 @@ type MovablePointElement = Extract<
PerseusInteractionElement,
{type: "movable-point"}
>;
const parseMovablePointType = constant("movable-point");
const parseMovablePointElement: Parser<MovablePointElement> = object({
type: parseMovablePointType,
key: string,
type: constant("movable-point"),
key: parseKey,
options: object({
startX: string,
startY: string,
Expand All @@ -122,10 +121,9 @@ type ParametricElement = Extract<
PerseusInteractionElement,
{type: "parametric"}
>;
const parseParametricType = constant("parametric");
const parseParametricElement: Parser<ParametricElement> = object({
type: parseParametricType,
key: string,
type: constant("parametric"),
key: parseKey,
options: object({
x: string,
y: string,
Expand All @@ -138,10 +136,9 @@ const parseParametricElement: Parser<ParametricElement> = object({
});

type PointElement = Extract<PerseusInteractionElement, {type: "point"}>;
const parsePointType = constant("point");
const parsePointElement: Parser<PointElement> = object({
type: parsePointType,
key: string,
type: constant("point"),
key: parseKey,
options: object({
color: string,
coordX: string,
Expand All @@ -150,10 +147,9 @@ const parsePointElement: Parser<PointElement> = object({
});

type RectangleElement = Extract<PerseusInteractionElement, {type: "rectangle"}>;
const parseRectangleType = constant("rectangle");
const parseRectangleElement: Parser<RectangleElement> = object({
type: parseRectangleType,
key: string,
type: constant("rectangle"),
key: parseKey,
options: object({
color: string,
coordX: string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,8 @@ const parseLockedLineType: Parser<LockedLineType> = object({
points: pair(parseLockedPointType, parseLockedPointType),
color: parseLockedFigureColor,
lineStyle: parseLockedLineStyle,
showPoint1: boolean,
showPoint2: boolean,
showPoint1: defaulted(boolean, () => false),
showPoint2: defaulted(boolean, () => false),
// TODO(benchristel): default labels to empty array?
labels: optional(array(parseLockedLabelType)),
ariaLabel: optional(string),
Expand Down Expand Up @@ -266,13 +266,14 @@ const parseLockedFunctionType: Parser<LockedFunctionType> = object({
ariaLabel: optional(string),
});

const parseLockedFigure: Parser<LockedFigure> = union(parseLockedPointType)
.or(parseLockedLineType)
.or(parseLockedVectorType)
.or(parseLockedEllipseType)
.or(parseLockedPolygonType)
.or(parseLockedFunctionType)
.or(parseLockedLabelType).parser;
const parseLockedFigure: Parser<LockedFigure> = discriminatedUnionOn("type")
.withBranch("point", parseLockedPointType)
.withBranch("line", parseLockedLineType)
.withBranch("vector", parseLockedVectorType)
.withBranch("ellipse", parseLockedEllipseType)
.withBranch("polygon", parseLockedPolygonType)
.withBranch("function", parseLockedFunctionType)
.withBranch("label", parseLockedLabelType).parser;

export const parseInteractiveGraphWidget: Parser<InteractiveGraphWidget> =
parseWidget(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,14 @@ import type {MeasurerWidget} from "@khanacademy/perseus-core";
export const parseMeasurerWidget: Parser<MeasurerWidget> = parseWidget(
constant("measurer"),
object({
image: parsePerseusImageBackground,
// The default value for image comes from measurer.tsx.
// See parse-perseus-json/README.md for why we want to duplicate the
// defaults here.
image: defaulted(parsePerseusImageBackground, () => ({
url: null,
top: 0,
left: 0,
})),
showProtractor: boolean,
showRuler: boolean,
rulerLabel: string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,15 @@ export const parsePlotterWidget: Parser<PlotterWidget> = parseWidget(
categories: array(string),
type: enumeration(...plotterPlotTypes),
maxY: number,
scaleY: number,
// The default value for scaleY comes from plotter.tsx.
// See parse-perseus-json/README.md for why we want to duplicate the
// defaults here.
scaleY: defaulted(number, () => 1),
labelInterval: optional(nullable(number)),
snapsPerLine: number,
// The default value for snapsPerLine comes from plotter.tsx.
// See parse-perseus-json/README.md for why we want to duplicate the
// defaults here.
snapsPerLine: defaulted(number, () => 2),
starting: array(number),
correct: array(number),
picUrl: optional(nullable(string)),
Expand Down
Loading

0 comments on commit bbf7f3b

Please sign in to comment.