Skip to content
Open
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
05579e3
[maximum-overline] Remove Interactive Graph Builder and refactor to u…
SonicScrewdriver Mar 25, 2026
1c78483
[maximum-overline] docs(changeset): Removing Interactive Graph Builde…
SonicScrewdriver Mar 25, 2026
f57524d
[maximum-overline] I HAVE FINALLY FIXED MY LINTER. HUZZAH.
SonicScrewdriver Mar 25, 2026
36d0cde
[maximum-overline] review fixes
SonicScrewdriver Mar 25, 2026
9587748
[maximum-overline] cleanup testdata
SonicScrewdriver Mar 25, 2026
e4db322
[maximum-overline] welp now I have a prettier issue.
SonicScrewdriver Mar 25, 2026
33b8350
[maximum-overline] PR review
SonicScrewdriver Mar 26, 2026
8bd3a88
[maximum-overline] Address PR feedback + cleanup comment
SonicScrewdriver Mar 26, 2026
ff121fc
[maximum-overline] fix match
SonicScrewdriver Mar 27, 2026
6a5f6e8
[maximum-overline] match old builder gridstep/snapstep
SonicScrewdriver Mar 27, 2026
6362242
[maximum-overline] claude
SonicScrewdriver Mar 27, 2026
3706ba0
[maximum-overline] sure fine
SonicScrewdriver Mar 27, 2026
78173e9
[maximum-overline] mention the generator (pr feedback)
SonicScrewdriver Apr 8, 2026
7f59b85
[maximum-overline] Remove angle again.
SonicScrewdriver Apr 8, 2026
a05ade3
[maximum-overline] PR feedback
SonicScrewdriver Apr 9, 2026
1112edd
Merge branch 'main' into maximum-overline
jeresig Apr 9, 2026
6a7c31b
Add extra domains.
jeresig Apr 9, 2026
0aa10dd
Handle Jest extra domain.
jeresig Apr 9, 2026
4923c64
[maximum-overline] updating comment to unstick frozen job
SonicScrewdriver Apr 9, 2026
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
7 changes: 7 additions & 0 deletions .changeset/lazy-donuts-yell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@khanacademy/perseus": patch
"@khanacademy/perseus-core": patch
"@khanacademy/perseus-editor": patch
---

Removing Interactive Graph Builder and refactoring tests/storybook to use more standardized generator patterns.
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ module.exports = {
"*.cypress.ts",
"*.test.ts",
"*.test.tsx",
"*.testdata.ts",
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It made sense to me to add a global override for our test data as it seems reasonable for these files to be exempt, but I'm also happy to remove this and re-add the file-specific override for interactive-graph.testdata.ts.

"*.stories.ts",
"*.stories.tsx",
"**/__testutils__/**",
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/node-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ jobs:
timeout-minutes: 5
with:
# registry.npmjs.org: npm package registry for pnpm install
extra-domains: registry.npmjs.org
# cdn.jsdelivr.net: speech-engine-rules files are hosted here
extra-domains: registry.npmjs.org cdn.jsdelivr.net

- name: Install & cache node_modules
uses: ./.github/actions/shared-node-cache
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@ jobs:
timeout-minutes: 5
with:
# registry.npmjs.org: npm package registry for pnpm install, npm upgrade, and publishing snapshot packages
extra-domains: registry.npmjs.org
# fulcio.sigstore.dev,rekor.sigstore.dev: signing certificates are stored here
extra-domains: registry.npmjs.org fulcio.sigstore.dev rekor.sigstore.dev

- name: Ensure main branch is available
run: |
Expand Down
3 changes: 3 additions & 0 deletions packages/perseus-core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,10 @@ export {
generateIGLockedPolygon,
generateIGLockedFunction,
generateIGLockedLabel,
generateIGExponentialGraph,
generateIGAbsoluteValueGraph,
generateInteractiveGraphWidget,
generateInteractiveGraphQuestion,
} from "./utils/generators/interactive-graph-widget-generator";
export {
generateNumericInputOptions,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import {
generateIGAbsoluteValueGraph,
generateIGAngleGraph,
generateIGCircleGraph,
generateIGExponentialGraph,
generateIGLinearGraph,
generateIGLinearSystemGraph,
generateIGLogarithmGraph,
Expand All @@ -20,6 +22,7 @@ import {
generateIGSinusoidGraph,
generateIGTangentGraph,
generateInteractiveGraphOptions,
generateInteractiveGraphQuestion,
generateInteractiveGraphWidget,
} from "./interactive-graph-widget-generator";

Expand Down Expand Up @@ -1332,3 +1335,149 @@ describe("generateIGLockedLabel", () => {
});
});
});

describe("generateIGExponentialGraph", () => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Decided to round out the tests for the new graphs while I was in here.

it("builds a default exponential graph", () => {
// Arrange, Act
const exponentialGraph = generateIGExponentialGraph();

// Assert
expect(exponentialGraph).toEqual({
type: "exponential",
});
});

it("builds an exponential graph with all props", () => {
// Arrange, Act
const exponentialGraph = generateIGExponentialGraph({
coords: [
[0, 1],
[1, 3],
],
asymptote: 0,
startCoords: {
coords: [
[0, 1],
[1, 3],
],
asymptote: 0,
},
});

// Assert
expect(exponentialGraph).toEqual({
type: "exponential",
coords: [
[0, 1],
[1, 3],
],
asymptote: 0,
startCoords: {
coords: [
[0, 1],
[1, 3],
],
asymptote: 0,
},
});
});
});

describe("generateIGAbsoluteValueGraph", () => {
it("builds a default absolute-value graph", () => {
// Arrange, Act
const absoluteValueGraph = generateIGAbsoluteValueGraph();

// Assert
expect(absoluteValueGraph).toEqual({
type: "absolute-value",
});
});

it("builds an absolute-value graph with all props", () => {
// Arrange, Act
const absoluteValueGraph = generateIGAbsoluteValueGraph({
coords: [
[0, 0],
[1, 1],
],
startCoords: [
[0, 0],
[1, 1],
],
});

// Assert
expect(absoluteValueGraph).toEqual({
type: "absolute-value",
coords: [
[0, 0],
[1, 1],
],
startCoords: [
[0, 0],
[1, 1],
],
});
});
});

describe("generateInteractiveGraphQuestion", () => {
it("builds a default interactive graph question", () => {
// Arrange, Act
const question = generateInteractiveGraphQuestion();

// Assert
expect(question.content).toBe("[[☃ interactive-graph 1]]");
expect(question.widgets["interactive-graph 1"]).toBeDefined();
expect(question.widgets["interactive-graph 1"].type).toBe(
"interactive-graph",
);
});

it("infers graph type from correct answer type", () => {
// Arrange, Act
const question = generateInteractiveGraphQuestion({
correct: generateIGPointGraph({numPoints: 3}),
});

// Assert
const options = question.widgets["interactive-graph 1"].options;
expect(options.correct.type).toBe("point");
expect(options.graph.type).toBe("point");
});

it("does not override graph when explicitly provided", () => {
// Arrange, Act
const question = generateInteractiveGraphQuestion({
correct: generateIGPointGraph({numPoints: 3}),
graph: generateIGPointGraph({numPoints: 5}),
});

// Assert
const options = question.widgets["interactive-graph 1"].options;
expect(options.graph).toEqual({type: "point", numPoints: 5});
});

it("sets static mode when isStatic is true", () => {
// Arrange, Act
const question = generateInteractiveGraphQuestion({
isStatic: true,
});

// Assert
expect(question.widgets["interactive-graph 1"].static).toBe(true);
});

it("uses custom content when provided", () => {
// Arrange, Act
const question = generateInteractiveGraphQuestion({
content: "Custom content [[☃ interactive-graph 1]]",
});

// Assert
expect(question.content).toBe(
"Custom content [[☃ interactive-graph 1]]",
);
});
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import interactiveGraphWidgetLogic from "../../widgets/interactive-graph";
import {getDefaultFigureForType} from "../get-default-figure-for-type";
import {generateTestPerseusRenderer} from "../test-utils";

import type {
InteractiveGraphWidget,
Expand All @@ -10,8 +11,11 @@ import type {
LockedPointType,
LockedPolygonType,
LockedVectorType,
PerseusGraphType,
PerseusGraphTypeAbsoluteValue,
PerseusGraphTypeAngle,
PerseusGraphTypeCircle,
PerseusGraphTypeExponential,
PerseusGraphTypeLinear,
PerseusGraphTypeLinearSystem,
PerseusGraphTypeLogarithm,
Expand All @@ -24,6 +28,7 @@ import type {
PerseusGraphTypeSinusoid,
PerseusGraphTypeTangent,
PerseusInteractiveGraphWidgetOptions,
PerseusRenderer,
} from "../../data-schema";

export function generateInteractiveGraphWidget(
Expand Down Expand Up @@ -52,7 +57,7 @@ export function generateInteractiveGraphOptions(
}

export function generateIGAngleGraph(
options?: Partial<PerseusGraphTypeAngle>,
options?: Partial<Omit<PerseusGraphTypeAngle, "type">>,
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is just some additional type safety that brings these generators inline with the implementation of the rest of the IG generators.

): PerseusGraphTypeAngle {
return {
type: "angle",
Expand All @@ -61,7 +66,7 @@ export function generateIGAngleGraph(
}

export function generateIGCircleGraph(
options?: Partial<PerseusGraphTypeCircle>,
options?: Partial<Omit<PerseusGraphTypeCircle, "type">>,
): PerseusGraphTypeCircle {
return {
type: "circle",
Expand All @@ -70,7 +75,7 @@ export function generateIGCircleGraph(
}

export function generateIGLinearGraph(
options?: Partial<PerseusGraphTypeLinear>,
options?: Partial<Omit<PerseusGraphTypeLinear, "type">>,
): PerseusGraphTypeLinear {
return {
type: "linear",
Expand All @@ -79,7 +84,7 @@ export function generateIGLinearGraph(
}

export function generateIGLinearSystemGraph(
options?: Partial<PerseusGraphTypeLinearSystem>,
options?: Partial<Omit<PerseusGraphTypeLinearSystem, "type">>,
): PerseusGraphTypeLinearSystem {
return {
type: "linear-system",
Expand Down Expand Up @@ -227,3 +232,63 @@ export function generateIGLockedLabel(
...options,
};
}

export function generateIGExponentialGraph(
options?: Partial<Omit<PerseusGraphTypeExponential, "type">>,
): PerseusGraphTypeExponential {
return {
type: "exponential",
...options,
};
}

export function generateIGAbsoluteValueGraph(
options?: Partial<Omit<PerseusGraphTypeAbsoluteValue, "type">>,
): PerseusGraphTypeAbsoluteValue {
return {
type: "absolute-value",
...options,
};
}

export function generateInteractiveGraphQuestion(
options?: Partial<PerseusInteractiveGraphWidgetOptions> & {
content?: string;
isStatic?: boolean;
},
): PerseusRenderer {
const {content, isStatic, ...widgetOptions} = options ?? {};

// The `graph` and `correct` fields share all fields except for
// the answers (coords, center, etc.) When only `correct` is provided,
// derive `graph` from it by stripping the answer-specific fields.
// This allows us to keep our test data more succinct.
if (widgetOptions.correct && !widgetOptions.graph) {
const {
coords: _,
coord: __,
center: ___,
radius: ____,
asymptote: _____,
match: ______,
...graphConfig
} = widgetOptions.correct as Record<string, unknown>;
Comment on lines +253 to +275
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 The developer architecture notes in logarithm.md (line 322) and tangent.md (line 187) still reference interactive-graph-question-builder.ts, which this PR deletes. Developers following these notes to implement a new graph type will hit a broken file reference; update both notes to point to the replacement module interactive-graph-widget-generator.ts.

Extended reasoning...

What the bug is and how it manifests

This PR deletes packages/perseus/src/widgets/interactive-graphs/interactive-graph-question-builder.ts and replaces it with packages/perseus-core/src/utils/generators/interactive-graph-widget-generator.ts. However, two developer architecture notes — logarithm.md and tangent.md — were not updated alongside this deletion. Both notes contain explicit file-path references that are now dead.

  • packages/perseus/src/widgets/interactive-graphs/__docs__/notes/logarithm.md line 322 references ...interactive-graph-question-builder.ts — withLogarithm(), LogarithmGraphConfig
  • packages/perseus/src/widgets/interactive-graphs/__docs__/notes/tangent.md line 187 references ...interactive-graph-question-builder.ts — Builder

The specific code path that triggers it

These notes are static Markdown documentation files intended to guide developers implementing new graph types. They list the files and functions a developer needs to touch, with interactive-graph-question-builder.ts called out as containing the builder class. Since that file no longer exists in the repository after this PR, any developer following the guide will look for a file that is simply gone.

Why existing code doesn't prevent it

The deletion of the builder was a source-code change; there is no automated tooling that cross-checks prose documentation for file-path accuracy. The notes were written to describe the old architecture and were not updated in this refactor.

What the impact would be

A developer tasked with adding a new interactive graph type (e.g., logarithm or tangent) would consult these notes, encounter the dead reference, and either waste time searching for the file or be left confused about where the builder logic now lives. The correct file is packages/perseus-core/src/utils/generators/interactive-graph-widget-generator.ts, which is where all the new generator functions (e.g., generateIGExponentialGraph, generateIGAbsoluteValueGraph) live.

How to fix it

In both logarithm.md:322 and tangent.md:187, replace the reference to interactive-graph-question-builder.ts with packages/perseus-core/src/utils/generators/interactive-graph-widget-generator.ts and update the function/class names cited to match the new generator API (e.g., the generator functions instead of withLogarithm()/LogarithmGraphConfig/Builder).

Step-by-step proof

  1. The PR diff shows interactive-graph-question-builder.ts deleted (entire file removed with --- a/.../interactive-graph-question-builder.ts diff header).
  2. logarithm.md line 322 contains the literal string interactive-graph-question-builder.ts — withLogarithm(), LogarithmGraphConfig — a path that no longer resolves.
  3. tangent.md line 187 contains the literal string interactive-graph-question-builder.ts — Builder — also a dead path.
  4. The new replacement module interactive-graph-widget-generator.ts is added by this PR and exports graph generators such as generateIGTangentGraph, generateIGExponentialGraph, etc.
  5. Neither doc file appears in the PR diff, confirming neither was updated.

widgetOptions.graph = graphConfig as PerseusGraphType;
}

const optionsWithDefaults = {
gridStep: [1, 1] as [number, number],
snapStep: [0.5, 0.5] as [number, number],
...widgetOptions,
};

return generateTestPerseusRenderer({
content: content ?? "[[☃ interactive-graph 1]]",
widgets: {
"interactive-graph 1": generateInteractiveGraphWidget({
static: isStatic ?? false,
options: generateInteractiveGraphOptions(optionsWithDefaults),
}),
},
});
}
Loading
Loading