Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move Radio scorer/validator #2106

Merged
merged 5 commits into from
Jan 17, 2025
Merged

Move Radio scorer/validator #2106

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8f33a6e
move radio scorer/validator
handeyeco Jan 14, 2025
0b88254
lint errorToString better
handeyeco Jan 15, 2025
f6de126
comment my unique type
handeyeco Jan 15, 2025
8a6e368
respond to Jeremy's feedback
handeyeco Jan 17, 2025
96fc81f
Move scoring logic: CSProgram, Iframe, and Dropdown (#2111)
handeyeco Jan 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions packages/perseus-score/src/error-codes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ const EXTRA_SYMBOLS_ERROR = "EXTRA_SYMBOLS_ERROR";
const WRONG_CASE_ERROR = "WRONG_CASE_ERROR";
const WRONG_LETTER_ERROR = "WRONG_LETTER_ERROR";
const MULTIPLICATION_SIGN_ERROR = "MULTIPLICATION_SIGN_ERROR";
const INVALID_SELECTION = "INVALID_SELECTION";
const INVALID_SELECTION_ERROR = "INVALID_SELECTION_ERROR";
const CHOOSE_CORRECT_NUM_ERROR = "CHOOSE_CORRECT_NUM_ERROR";
const NOT_NONE_ABOVE_ERROR = "NOT_NONE_ABOVE_ERROR";

const ErrorCodes = {
MISSING_PERCENT_ERROR,
Expand All @@ -15,7 +17,9 @@ const ErrorCodes = {
WRONG_CASE_ERROR,
WRONG_LETTER_ERROR,
MULTIPLICATION_SIGN_ERROR,
INVALID_SELECTION,
INVALID_SELECTION_ERROR,
CHOOSE_CORRECT_NUM_ERROR,
NOT_NONE_ABOVE_ERROR,
};

export default ErrorCodes;
1 change: 1 addition & 0 deletions packages/perseus-score/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export {default as ErrorCodes} from "./error-codes";
export type * from "./validation.types";
export {default as scoreCategorizer} from "./widgets/categorizer/score-categorizer";
export {default as scoreNumericInput} from "./widgets/numeric-input/score-numeric-input";
export {default as scoreRadio} from "./widgets/radio/score-radio";
export {
default as scoreInputNumber,
inputNumberAnswerTypes,
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus-score/src/widgets/categorizer/validate-categorizer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ describe("validateCategorizer", () => {
} as const;
const score = validateCategorizer(userInput, validationData);

expect(score).toHaveInvalidInput("INVALID_SELECTION");
expect(score).toHaveInvalidInput("INVALID_SELECTION_ERROR");
});

it("returns null if the userInput is valid", () => {
Expand Down
2 changes: 1 addition & 1 deletion packages/perseus-score/src/widgets/categorizer/validate-categorizer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ function validateCategorizer(
if (incomplete) {
return {
type: "invalid",
message: ErrorCodes.INVALID_SELECTION,
message: ErrorCodes.INVALID_SELECTION_ERROR,
};
}
return null;
Expand Down
20 changes: 9 additions & 11 deletions ...eus/src/widgets/radio/score-radio.test.ts → ...ore/src/widgets/radio/score-radio.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
import {mockStrings} from "../../strings";

import scoreRadio from "./score-radio";

import type {
PerseusRadioRubric,
PerseusRadioUserInput,
} from "@khanacademy/perseus-score";
} from "../../validation.types";

describe("scoreRadio", () => {
it("is invalid when number selected does not match number correct", () => {
Expand All @@ -22,7 +20,7 @@ describe("scoreRadio", () => {
],
};

const score = scoreRadio(userInput, rubric, mockStrings);
const score = scoreRadio(userInput, rubric);

expect(score).toHaveInvalidInput();
});
Expand All @@ -46,7 +44,7 @@ describe("scoreRadio", () => {
],
};

const score = scoreRadio(userInput, rubric, mockStrings);
const score = scoreRadio(userInput, rubric);

expect(score).toHaveInvalidInput();
});
Expand All @@ -65,7 +63,7 @@ describe("scoreRadio", () => {
],
};

const score = scoreRadio(userInput, rubric, mockStrings);
const score = scoreRadio(userInput, rubric);

expect(score).toHaveBeenAnsweredCorrectly();
});
Expand All @@ -84,7 +82,7 @@ describe("scoreRadio", () => {
],
};

const score = scoreRadio(userInput, rubric, mockStrings);
const score = scoreRadio(userInput, rubric);

expect(score).toHaveBeenAnsweredIncorrectly();
});
Expand All @@ -103,7 +101,7 @@ describe("scoreRadio", () => {
],
};

const score = scoreRadio(userInput, rubric, mockStrings);
const score = scoreRadio(userInput, rubric);

expect(score).toHaveBeenAnsweredCorrectly();
});
Expand All @@ -122,7 +120,7 @@ describe("scoreRadio", () => {
],
};

const score = scoreRadio(userInput, rubric, mockStrings);
const score = scoreRadio(userInput, rubric);

expect(score).toHaveBeenAnsweredIncorrectly();
});
Expand All @@ -142,7 +140,7 @@ describe("scoreRadio", () => {
],
};

const score = scoreRadio(userInput, rubric, mockStrings);
const score = scoreRadio(userInput, rubric);

expect(score).toHaveBeenAnsweredCorrectly();
});
Expand All @@ -162,7 +160,7 @@ describe("scoreRadio", () => {
],
};

const score = scoreRadio(userInput, rubric, mockStrings);
const score = scoreRadio(userInput, rubric);

expect(score).toHaveBeenAnsweredIncorrectly();
});
Expand Down
12 changes: 6 additions & 6 deletions .../perseus/src/widgets/radio/score-radio.ts → ...us-score/src/widgets/radio/score-radio.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import ErrorCodes from "../../error-codes";

import validateRadio from "./validate-radio";

import type {PerseusStrings} from "../../strings";
import type {
PerseusScore,
PerseusRadioRubric,
PerseusRadioUserInput,
} from "@khanacademy/perseus-score";
PerseusScore,
} from "../../validation.types";

function scoreRadio(
userInput: PerseusRadioUserInput,
rubric: PerseusRadioRubric,
strings: PerseusStrings,
): PerseusScore {
const validationError = validateRadio(userInput);
if (validationError) {
Expand All @@ -28,7 +28,7 @@ function scoreRadio(
if (numCorrect > 1 && numSelected !== numCorrect) {
return {
type: "invalid",
message: strings.chooseCorrectNum,
message: ErrorCodes.CHOOSE_CORRECT_NUM_ERROR,
};
// If NOTA and some other answer are checked, ...
}
Expand All @@ -41,7 +41,7 @@ function scoreRadio(
if (noneOfTheAboveSelected && numSelected > 1) {
return {
type: "invalid",
message: strings.notNoneOfTheAbove,
message: ErrorCodes.NOT_NONE_ABOVE_ERROR,
};
}

Expand Down
2 changes: 1 addition & 1 deletion .../src/widgets/radio/validate-radio.test.ts → .../src/widgets/radio/validate-radio.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import validateRadio from "./validate-radio";

import type {PerseusRadioUserInput} from "@khanacademy/perseus-score";
import type {PerseusRadioUserInput} from "../../validation.types";

describe("validateRadio", () => {
it("is invalid when no options are selected", () => {
Expand Down
2 changes: 1 addition & 1 deletion ...rseus/src/widgets/radio/validate-radio.ts → ...score/src/widgets/radio/validate-radio.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type {
PerseusRadioUserInput,
ValidationResult,
} from "@khanacademy/perseus-score";
} from "../../validation.types";

/**
* Checks if the user has selected at least one option. Additional validation
Expand Down
29 changes: 18 additions & 11 deletions packages/perseus/src/strings.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {ErrorCodes} from "@khanacademy/perseus-score";
import type {ErrorCodes} from "@khanacademy/perseus-score";

/**
* The translated strings that are used to render Perseus.
Expand Down Expand Up @@ -700,17 +700,24 @@ export const mockStrings: PerseusStrings = {
// The above strings are used for interactive graph SR descriptions.
};

const errorToString = {
[ErrorCodes.MISSING_PERCENT_ERROR]: strings.MISSING_PERCENT_ERROR,
[ErrorCodes.NEEDS_TO_BE_SIMPLIFIED_ERROR]:
strings.NEEDS_TO_BE_SIMPLFIED_ERROR,
[ErrorCodes.APPROXIMATED_PI_ERROR]: strings.APPROXIMATED_PI_ERROR,
[ErrorCodes.EXTRA_SYMBOLS_ERROR]: strings.EXTRA_SYMBOLS_ERROR,
[ErrorCodes.WRONG_CASE_ERROR]: strings.WRONG_CASE_ERROR,
[ErrorCodes.WRONG_LETTER_ERROR]: strings.WRONG_LETTER_ERROR,
[ErrorCodes.MULTIPLICATION_SIGN_ERROR]: strings.MULTIPLICATION_SIGN_ERROR,
[ErrorCodes.INVALID_SELECTION]: strings.invalidSelection,
// This type helps us make sure all error codes are mapped to strings
type ErrorStringMap = {
[K in keyof typeof ErrorCodes]: string;
};

const errorToString: ErrorStringMap = {
MISSING_PERCENT_ERROR: strings.MISSING_PERCENT_ERROR as string,
NEEDS_TO_BE_SIMPLIFIED_ERROR: strings.NEEDS_TO_BE_SIMPLFIED_ERROR as string,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of being forced to as string each of these, I think we can fix this more cleanly by moving the type annotation for strings from:

export const strings: {
    [key in keyof PerseusStrings]:
        | string
        | {context?: string; message: string}
        | {context?: string; one: string; other: string};
} = {
    closeKeypad: "close math keypad",
    ...
};

to:

export const strings = {
    closeKeypad: "close math keypad",
    ...
} satisfies {
    [key in keyof PerseusStrings]:
        | string
        | {context?: string; message: string}
        | {context?: string; one: string; other: string};
};

Using satisfies instead of : mytype = means that we allow TypeScript to infer the type of each key in strings, but the entire structure must conform to the satisfied type. So TypeScript can now infer that strings has a closeKeypad key with a string value instead of a value that is one of string {context..} etc.

APPROXIMATED_PI_ERROR: strings.APPROXIMATED_PI_ERROR as string,
EXTRA_SYMBOLS_ERROR: strings.EXTRA_SYMBOLS_ERROR as string,
WRONG_CASE_ERROR: strings.WRONG_CASE_ERROR as string,
WRONG_LETTER_ERROR: strings.WRONG_LETTER_ERROR as string,
MULTIPLICATION_SIGN_ERROR: strings.MULTIPLICATION_SIGN_ERROR as string,
INVALID_SELECTION_ERROR: strings.invalidSelection as string,
CHOOSE_CORRECT_NUM_ERROR: strings.chooseCorrectNum as string,
NOT_NONE_ABOVE_ERROR: strings.notNoneOfTheAbove as string,
};

export function mapErrorToString(err: string | null | undefined) {
if (!err) {
return err;
Expand Down
4 changes: 2 additions & 2 deletions packages/perseus/src/widgets/categorizer/categorizer.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ describe("categorizer widget", () => {
// Assert
expect(score).toMatchInlineSnapshot(`
{
"message": "INVALID_SELECTION",
"message": "INVALID_SELECTION_ERROR",
"type": "invalid",
}
`);
Expand All @@ -94,7 +94,7 @@ describe("categorizer widget", () => {
// assert
expect(score).toMatchInlineSnapshot(`
{
"message": "INVALID_SELECTION",
"message": "INVALID_SELECTION_ERROR",
"type": "invalid",
}
`);
Expand Down
19 changes: 9 additions & 10 deletions packages/perseus/src/widgets/radio/__tests__/radio.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,16 +1,18 @@
import {describe, beforeEach, it} from "@jest/globals";
import {
scoreRadio,
type PerseusRadioUserInput,
} from "@khanacademy/perseus-score";
import {act, screen, fireEvent, waitFor} from "@testing-library/react";
import {userEvent as userEventLib} from "@testing-library/user-event";

import {clone} from "../../../../../../testing/object-utils";
import {testDependencies} from "../../../../../../testing/test-dependencies";
import * as Dependencies from "../../../dependencies";
import {mockStrings} from "../../../strings";
import {scorePerseusItemTesting} from "../../../util/test-utils";
import {renderQuestion} from "../../__testutils__/renderQuestion";
import PassageWidget from "../../passage";
import RadioWidgetExport from "../radio";
import scoreRadio from "../score-radio";

import {
questionAndAnswer,
Expand All @@ -24,7 +26,6 @@ import type {
PerseusRadioWidgetOptions,
PerseusRenderer,
} from "@khanacademy/perseus-core";
import type {PerseusRadioUserInput} from "@khanacademy/perseus-score";
import type {UserEvent} from "@testing-library/user-event";

const selectOption = async (
Expand Down Expand Up @@ -901,9 +902,7 @@ describe("Radio Widget", () => {
);

// Assert
expect(score).toHaveInvalidInput(
"Please choose the correct number of answers",
);
expect(score).toHaveInvalidInput("CHOOSE_CORRECT_NUM_ERROR");
});

it.each(incorrect)(
Expand Down Expand Up @@ -967,7 +966,7 @@ describe("Radio Widget", () => {
const userInput =
renderer.getUserInput()[0] as PerseusRadioUserInput;
const rubric = shuffledQuestion.widgets["radio 1"].options;
const widgetScore = scoreRadio(userInput, rubric, mockStrings);
const widgetScore = scoreRadio(userInput, rubric);
const rendererScore = scorePerseusItemTesting(
shuffledQuestion,
renderer.getUserInputMap(),
Expand Down Expand Up @@ -995,7 +994,7 @@ describe("Radio Widget", () => {
const userInput =
renderer.getUserInput()[0] as PerseusRadioUserInput;
const rubric = shuffledQuestion.widgets["radio 1"].options;
const widgetScore = scoreRadio(userInput, rubric, mockStrings);
const widgetScore = scoreRadio(userInput, rubric);
const rendererScore = scorePerseusItemTesting(
shuffledQuestion,
renderer.getUserInputMap(),
Expand Down Expand Up @@ -1023,7 +1022,7 @@ describe("Radio Widget", () => {
const userInput =
renderer.getUserInput()[0] as PerseusRadioUserInput;
const rubric = shuffledNoneQuestion.widgets["radio 1"].options;
const widgetScore = scoreRadio(userInput, rubric, mockStrings);
const widgetScore = scoreRadio(userInput, rubric);
const rendererScore = scorePerseusItemTesting(
shuffledNoneQuestion,
renderer.getUserInputMap(),
Expand Down Expand Up @@ -1051,7 +1050,7 @@ describe("Radio Widget", () => {
const userInput =
renderer.getUserInput()[0] as PerseusRadioUserInput;
const rubric = shuffledNoneQuestion.widgets["radio 1"].options;
const widgetScore = scoreRadio(userInput, rubric, mockStrings);
const widgetScore = scoreRadio(userInput, rubric);
const rendererScore = scorePerseusItemTesting(
shuffledQuestion,
renderer.getUserInputMap(),
Expand Down
16 changes: 6 additions & 10 deletions packages/perseus/src/widgets/radio/radio-component.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import {linterContextDefault} from "@khanacademy/perseus-linter";
import {
scoreRadio,
type PerseusRadioRubric,
type PerseusRadioUserInput,
} from "@khanacademy/perseus-score";
import * as React from "react";

import {PerseusI18nContext} from "../../components/i18n-context";
Expand All @@ -8,7 +13,6 @@ import {getPromptJSON as _getPromptJSON} from "../../widget-ai-utils/radio/radio
import PassageRef from "../passage-ref/passage-ref";

import BaseRadio from "./base-radio";
import scoreRadio from "./score-radio";

import type {FocusFunction, ChoiceType} from "./base-radio";
import type {WidgetProps, ChoiceState, Widget} from "../../types";
Expand All @@ -18,10 +22,6 @@ import type {
PerseusRadioWidgetOptions,
ShowSolutions,
} from "@khanacademy/perseus-core";
import type {
PerseusRadioRubric,
PerseusRadioUserInput,
} from "@khanacademy/perseus-score";

// RenderProps is the return type for radio.jsx#transform
export type RenderProps = {
Expand Down Expand Up @@ -271,11 +271,7 @@ class Radio extends React.Component<Props> implements Widget {
) => void = (rubric) => {
const {choiceStates} = this.props;
if (choiceStates) {
const score = scoreRadio(
this.getUserInput(),
rubric,
this.context.strings,
);
const score = scoreRadio(this.getUserInput(), rubric);
const widgetCorrect =
score.type === "points" && score.total === score.earned;

Expand Down
2 changes: 1 addition & 1 deletion packages/perseus/src/widgets/radio/radio.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import {scoreRadio} from "@khanacademy/perseus-score";
import _ from "underscore";

import Util from "../../util";

import Radio from "./radio-component";
import scoreRadio from "./score-radio";

import type {RenderProps, RadioChoiceWithMetadata} from "./radio-component";
import type {PerseusStrings} from "../../strings";
Expand Down
Loading