Skip to content

[ Interactive Graph | Exponential Graph ] PR2: State Management#3377

Merged
SonicScrewdriver merged 10 commits intomainfrom
LEMS-3711/exponential-pr2
Mar 23, 2026
Merged

[ Interactive Graph | Exponential Graph ] PR2: State Management#3377
SonicScrewdriver merged 10 commits intomainfrom
LEMS-3711/exponential-pr2

Conversation

@SonicScrewdriver
Copy link
Copy Markdown
Contributor

@SonicScrewdriver SonicScrewdriver commented Mar 18, 2026

Summary:

This PR was created with the help of AI, albeit with heavy oversight and review.

This is part of a series of PRs implementing the exponential graph type for
the Interactive Graph widget:

PR1 – type definitions and schema
▶️ PR2 – state management (this PR)
PR3 – rendering & accessibility
PR4 – scoring
PR5 – editor support

Issue: LEMS-3711

  • Added state initialization, actions, and reducer logic for the exponential graph type

  • Implements the full state management layer for exponential graphs. No rendering
    is wired up yet — the "exponential" case in mafs-graph.tsx still throws
    "Not implemented", so there is no visible change to users.

Changes:

  • initialize-graph-state.tsinitializeGraphState now handles "exponential"
    via a new getExponentialCoords() helper, which reads explicit coords and
    asymptote from the graph config, falls back to startCoords if set, and
    otherwise defaults to two normalized points above the x-axis with the asymptote
    at y=0.
  • interactive-graph-action.ts — Registers exponential: { movePoint, moveCenter }
    in the actions namespace.
  • interactive-graph-reducer.ts — Implements doMovePoint and doMoveCenter
    for exponential graphs:
    • doMovePoint: bounds and snaps the destination, prevents landing on the
      asymptote or sharing an x-value with the other point. If a point is dragged
      past the asymptote, the other point is reflected across it (mirrors the
      logarithm reducer, preserving the constraint that both points stay on the
      same side).
    • doMoveCenter: moves the asymptote vertically only. If the new y-value would
      land between the curve points, snaps to the nearest valid side. Also blocks
      the move if the asymptote would coincide exactly with either point.
  • interactive-graph-state.tsgetGradableGraph extracts coords and
    asymptote from exponential state for scoring.
  • mafs-state-to-interactive-graph.ts — Handles the "exponential" case,
    mapping state coords and asymptote back to the graph config.
  • interactive-graph-question-builder.ts — Adds withExponential() builder
    method and ExponentialGraphConfig class. asymptote is a scalar number
    throughout, consistent with the schema from PR1.
  • interactive-graph.testdata.ts — Adds exponentialQuestion test fixture.
  • reducer/initialize-graph-state.test.ts — Tests for getExponentialCoords
    covering explicit coords, startCoords fallback, and default behavior.
  • reducer/interactive-graph-reducer.test.ts — Tests for doMovePoint and
    doMoveCenter, including invalid moves, asymptote snapping, and point
    reflection across the asymptote.
  • packages/perseus/src/index.ts — Exports getExponentialCoords.

Test plan:

  • pnpm test packages/perseus — all new reducer and initializer tests pass
  • pnpm tsc — no new type errors
  • pnpm lint — no lint errors
  • No existing interactive graph tests regress

Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

Claude Code Review

This repository is configured for manual code reviews. Comment @claude review to trigger a review.

@SonicScrewdriver SonicScrewdriver force-pushed the LEMS-3711/exponential-pr2 branch from dec1ebf to 255ff93 Compare March 18, 2026 17:43
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 18, 2026

🗄️ Schema Change: No Changes ✅

@SonicScrewdriver SonicScrewdriver changed the title Improving legibility/efficiency of asymptote [ Interactive Graph | Exponential Graph ] PR2: State Management Mar 18, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 18, 2026

🛠️ Item Splitting: No Changes ✅

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 18, 2026

Size Change: +712 B (+0.15%)

Total Size: 491 kB

Filename Size Change
packages/perseus/dist/es/index.js 191 kB +712 B (+0.37%)
ℹ️ View Unchanged
Filename Size
packages/kas/dist/es/index.js 20.5 kB
packages/keypad-context/dist/es/index.js 1 kB
packages/kmath/dist/es/index.js 6.21 kB
packages/math-input/dist/es/index.js 98.5 kB
packages/math-input/dist/es/strings.js 1.61 kB
packages/perseus-core/dist/es/index.item-splitting.js 11.9 kB
packages/perseus-core/dist/es/index.js 25 kB
packages/perseus-editor/dist/es/index.js 101 kB
packages/perseus-linter/dist/es/index.js 8.82 kB
packages/perseus-score/dist/es/index.js 9.5 kB
packages/perseus-utils/dist/es/index.js 403 B
packages/perseus/dist/es/strings.js 7.84 kB
packages/pure-markdown/dist/es/index.js 1.39 kB
packages/simple-markdown/dist/es/index.js 6.71 kB

compressed-size-action

@SonicScrewdriver SonicScrewdriver force-pushed the LEMS-3711/exponential-pr2 branch 3 times, most recently from 9423f62 to 322259f Compare March 18, 2026 18:30
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 18, 2026

npm Snapshot: Published

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

Example:

pnpm add @khanacademy/perseus@PR3377

If you are working in Khan Academy's frontend, you can run the below command.

./dev/tools/bump_perseus_version.ts -t PR3377

If you are working in Khan Academy's webapp, you can run the below command.

./dev/tools/bump_perseus_version.js -t PR3377

@SonicScrewdriver
Copy link
Copy Markdown
Contributor Author

@claude review

Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

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

All three previously reported bugs have been addressed — LGTM.

Extended reasoning...

Overview

This PR adds the state management layer for exponential graphs in the Interactive Graph widget, touching 10 files across initialization, actions, reducer logic, state extraction, state-to-graph conversion, question builder, and tests. It follows established patterns from existing graph types (sinusoid, circle, etc.).

Security risks

No security concerns — this is pure UI state management for an educational math widget with no auth, network, or data-handling implications.

Level of scrutiny

Moderate scrutiny was warranted given the non-trivial reducer logic (asymptote constraints, point reflection, bounds checking). My previous review identified three bugs, all of which have been fixed in commit 97ba787:

  1. Reflected point in doMovePoint now passes through boundAndSnapToGrid
  2. Asymptote Y in doMoveCenter now uses boundAndSnapToGrid for the initial value and clamp after snap correction
  3. ExponentialGraphConfig.graph() now defaults startAsymptote to 0 via nullish coalescing

Other factors

The PR includes good test coverage for initialization (3 tests) and reducer logic (6 tests covering point movement, asymptote movement, rejection cases, and interaction flags). The code closely mirrors existing patterns for sinusoid and circle graphs. No rendering is wired up yet (the exponential case in mafs-graph.tsx still throws), so there is no user-visible change.

});
});

const baseExponentialGraphState: InteractiveGraphState = {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

suggestion create a generator for this, see sample here

@SonicScrewdriver SonicScrewdriver force-pushed the LEMS-3711/exponential-pr1 branch from 47f69cc to e79613a Compare March 23, 2026 21:33
Comment on lines 19 to 22

export function getStartCoords(graph: PerseusGraphType): StartCoords {
if ("startCoords" in graph) {
if ("startCoords" in graph && graph.type !== "exponential") {
return graph.startCoords;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 In start-coords/util.ts, shouldShowStartCoordsUI now returns true for "exponential", but neither StartCoordsSettingsInner nor getDefaultGraphStartCoords handle the exponential case — so the editor renders a blank start-coords section with a "Use default start coordinates" button that calls changeStartCoords(undefined), silently wiping any configured startCoords. Compounding this, getStartCoords explicitly returns undefined for exponential (added in the same PR to avoid a TypeScript type mismatch), so serialize() always writes startCoords: undefined for exponential graphs, silently discarding saved start coordinates on every copy/paste or save. The tangent case carries an explicit TODO comment that should have been applied to exponential as well until PR5 lands full editor support.

Extended reasoning...

What the bug is

This PR introduces two mutually reinforcing problems in the editor start-coords pipeline, both rooted in the premature addition of "exponential" to shouldShowStartCoordsUI without corresponding support elsewhere.

Bug 1 — Blank UI with a destructive reset button.
shouldShowStartCoordsUI (util.ts line 199) now returns true for "exponential", causing StartCoordsSettings to render. StartCoordsSettings always renders both a StartCoordsSettingsInner component and a top-level "Use default start coordinates" button. StartCoordsSettingsInner has no case "exponential" and falls through to default: return null, producing a blank section. The reset button still renders and calls onChange(getDefaultGraphStartCoords(props, range, step)). getDefaultGraphStartCoords also has no case "exponential" and returns undefined, so pressing the button calls changeStartCoords(undefined), setting startCoords: undefined on the graph and erasing any previously-configured start coordinates.

Bug 2 — Silent data loss in editor serialization.
The same PR adds the guard graph.type !== "exponential" to getStartCoords (util.ts:19-22) to avoid a TypeScript type error (the exponential startCoords shape {coords: [Coord, Coord]; asymptote: number} is not part of the StartCoords union). This is a valid TypeScript fix, but no alternative serialization path is provided. serialize() in interactive-graph-editor.tsx uses getStartCoords(this.props.graph) as the sole source for startCoords in the serialized JSON, so for exponential graphs this always evaluates to undefined. Every serialize call (copy/paste, save) silently drops the start coordinates.

The specific code paths

  • shouldShowStartCoordsUI (util.ts:199): case "exponential": return true;
  • getStartCoords (util.ts:19-22): adds && graph.type !== "exponential" guard, returning undefined for all exponential graphs
  • StartCoordsSettingsInner (start-coords-settings.tsx:44-123): no case "exponential", falls to default: return null
  • getDefaultGraphStartCoords (util.ts:27-93): no case "exponential", falls to default: return undefined
  • serialize() (interactive-graph-editor.tsx:215): startCoords: this.props.graph && getStartCoords(this.props.graph) -- always undefined for exponential

Why existing code does not prevent it

The tangent case in shouldShowStartCoordsUI has an explicit guard explaining this exact situation:

This guard was not applied to exponential. The comment documents the correct pattern: shouldShowStartCoordsUI should remain false until the corresponding editor UI (PR5) is ready.

Step-by-step proof (Bug 1)

  1. Content author opens an exponential interactive graph in the editor.
  2. shouldShowStartCoordsUI returns true -> StartCoordsSettings renders.
  3. StartCoordsSettingsInner renders null (no case "exponential") -> blank input area.
  4. Author sees the "Use default start coordinates" button and clicks it.
  5. getDefaultGraphStartCoords returns undefined -> changeStartCoords(undefined) is called.
  6. The graph prop now has startCoords: undefined, erasing any previously-set start coordinates.

Step-by-step proof (Bug 2)

  1. An exponential graph exists with startCoords: {coords: [[1,2],[3,4]], asymptote: 0} (e.g., set via withExponential({startCoords: ...}) or a prior editor session).
  2. Author opens the graph in the editor; getExponentialCoords in initialize-graph-state.ts correctly reads startCoords to initialize state.
  3. Author makes any change and saves (or copies the widget).
  4. serialize() calls getStartCoords(this.props.graph) -> returns undefined because graph.type === "exponential".
  5. The serialized JSON contains startCoords: undefined.
  6. On next load, getExponentialCoords finds no startCoords and falls back to defaults -- the author configuration is permanently lost.

How to fix

The simplest fix follows the tangent precedent: keep "exponential" returning false in shouldShowStartCoordsUI until PR5 adds full editor support. For the serialization bug, the StartCoords union type must be extended to include the exponential shape, or serialize() must be given a separate code path for exponential that preserves graph.startCoords directly.

Comment on lines +578 to +597
if (movedSide !== otherSide) {
const reflectedY = 2 * asymptoteY - otherPoint[Y];
const updatedCoords: [vec.Vector2, vec.Vector2] = [
...state.coords,
];
updatedCoords[action.index] = boundDestination;
updatedCoords[otherIndex] = boundAndSnapToGrid(
[otherPoint[X], reflectedY],
state,
);
return {
...state,
hasBeenInteractedWith: true,
coords: updatedCoords,
};
}

return {
...state,
hasBeenInteractedWith: true,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 In doMovePoint for exponential graphs, after reflecting the other point across the asymptote, the result is passed through boundAndSnapToGrid but never checked against asymptoteY. If otherPoint[Y] is within half a snap step of the asymptote (e.g. otherPoint[Y] = 2.3, asymptoteY = 2, snapStep = 1), the reflection 1.7 rounds to 2.0, placing a curve point exactly on the asymptote — an invalid state not caught by the earlier guard (which only protects the moved point). Add a guard if (reflectedPoint[Y] === asymptoteY) { return state; } after computing the bounded reflection.

Extended reasoning...

What the bug is

In doMovePoint for "exponential" graphs (interactive-graph-reducer.ts, lines ~578–597), when the dragged point crosses the asymptote the code reflects the other point across it to keep both curve points on the same side. The reflected Y is computed as reflectedY = 2 * asymptoteY - otherPoint[Y], and then stored via:

updatedCoords[otherIndex] = boundAndSnapToGrid(
    [otherPoint[X], reflectedY],
    state,
);

The boundAndSnapToGrid function snaps to the nearest multiple of snapStep[Y]. If reflectedY is within 0.5 * snapStep[Y] of asymptoteY, it rounds to exactly asymptoteY, placing a curve point directly on the asymptote — a state the code explicitly forbids.

Why the existing guard does not prevent it

The guard if (boundDestination[Y] === asymptoteY) { return state; } only checks the moved point. There is no analogous check on updatedCoords[otherIndex] after the reflection is snapped. The snapping itself is what introduces the violation.

Step-by-step proof

  1. State: coords = [[0, 2.6], [2, 2.3]], asymptote = 2, snapStep = [1, 1], range = [[-10,10],[-10,10]].
  2. User drags point 0 to [0, 1].
  3. boundDestination = boundAndSnapToGrid([0, 1], state) = [0, 1].
  4. Check 1 !== 2 passes (moved point not on asymptote).
  5. movedSide = (1 > 2) = false; otherSide = (2.3 > 2) = true → sides differ, enter reflection branch.
  6. reflectedY = 2*2 - 2.3 = 1.7.
  7. boundAndSnapToGrid([2, 1.7], state) = snap([1,1], bound(..., [2, 1.7])) = [2, Math.round(1.7)] = [2, 2].
  8. updatedCoords[1] = [2, 2] but asymptoteY = 2curve point is on the asymptote.

How initial state can be non-grid-aligned

The getExponentialCoords function passes graph.startCoords.coords directly into state without snapping. Any startCoords with float Y-values (e.g. coords: [[0, 2.3], [3, 4.5]], asymptote: 2) produces a valid but non-grid-aligned initial state, making this scenario reachable in practice.

Impact

A curve point on the asymptote is geometrically invalid for an exponential function — getExponentialCoefficients returns undefined for this input. Downstream scoring and rendering (PR3+) would receive incoherent state that they may not handle gracefully.

Fix

Capture the reflected point before storing it and reject the move if it lands on the asymptote:

const reflectedPoint = boundAndSnapToGrid(
    [otherPoint[X], reflectedY],
    state,
);
if (reflectedPoint[Y] === asymptoteY) {
    return state; // reflection would land on asymptote; reject move
}
updatedCoords[otherIndex] = reflectedPoint;

@SonicScrewdriver SonicScrewdriver changed the base branch from LEMS-3711/exponential-pr1 to main March 23, 2026 22:17
@SonicScrewdriver SonicScrewdriver force-pushed the LEMS-3711/exponential-pr2 branch from 6a28c3d to 58a0135 Compare March 23, 2026 22:34
Comment on lines +567 to +594
if (newCoords[0][X] === newCoords[1][X]) {
return state;
}

// If the moved point crosses the asymptote, reflect the other
// point across it so the entire curve moves to the new side.
// This matches Grapher behavior where dragging past the asymptote
// relocates the whole curve. Mirrors the logarithm reducer.
const otherIndex = 1 - action.index;
const otherPoint = state.coords[otherIndex];
const movedSide = boundDestination[Y] > asymptoteY;
const otherSide = otherPoint[Y] > asymptoteY;

if (movedSide !== otherSide) {
const reflectedY = 2 * asymptoteY - otherPoint[Y];
const updatedCoords: [vec.Vector2, vec.Vector2] = [
...state.coords,
];
updatedCoords[action.index] = boundDestination;
updatedCoords[otherIndex] = boundAndSnapToGrid(
[otherPoint[X], reflectedY],
state,
);
return {
...state,
hasBeenInteractedWith: true,
coords: updatedCoords,
};
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔴 In doMovePoint for exponential graphs, the same-x guard at line 567 compares the snapped boundDestination[X] against the raw unsnapped otherPoint[X] from state. When the reflection branch fires and calls boundAndSnapToGrid([otherPoint[X], reflectedY]), snapping can round otherPoint[X] to the same value as boundDestination[X], leaving both curve points at the same x-coordinate and making the exponential coefficients undefined. Fix by adding a same-x check after computing updatedCoords[otherIndex] in the reflection branch.

Extended reasoning...

The bug

In the case "exponential" block of doMovePoint (lines 548–609 of interactive-graph-reducer.ts), the guard if (newCoords[0][X] === newCoords[1][X]) { return state; } fires before the reflection branch. At that point newCoords[action.index][X] holds the already-snapped boundDestination[X], but newCoords[otherIndex][X] is still the raw, unsnapped value from state.coords[otherIndex][X] (copied verbatim when the array was spread). The guard compares a snapped value against an unsnapped value, so it can pass even when the two would collide after snapping.

The problematic code path

When movedSide !== otherSide (the moved point crossed the asymptote), the reflection branch runs:

const reflectedY = 2 * asymptoteY - otherPoint[Y];
updatedCoords[otherIndex] = boundAndSnapToGrid(
    [otherPoint[X], reflectedY],
    state,
);

boundAndSnapToGrid calls snap(snapStep, bound(...)), which rounds otherPoint[X] to the nearest multiple of snapStep[X]. If that rounded value equals boundDestination[X], both points land at the same x-coordinate with no guard catching it.

Why existing code does not prevent it

The only same-x check is the pre-reflection guard that compares unsnapped-vs-snapped values. There is no post-reflection guard. getExponentialCoords stores graph.startCoords.coords directly without snapping, so a content author who supplies non-integer x-values (e.g. [[1.6, 5], [3, 3]]) creates a valid but non-grid-aligned initial state, making the scenario fully reachable.

Step-by-step proof

  1. State: coords = [[1.6, 5], [3, 3]], asymptote = 1, snapStep = [1, 1], range = [[-10,10],[-10,10]].
  2. User drags point 1 to [2, -1]. boundDestination = boundAndSnapToGrid([2,-1]) = [2, -1].
  3. newCoords = [[1.6, 5], [2, -1]].
  4. Same-x guard: 1.6 !== 2 → passes.
  5. movedSide = (-1 > 1) = false; otherSide = (5 > 1) = true → sides differ, reflection fires.
  6. reflectedY = 2*1 - 5 = -3.
  7. boundAndSnapToGrid([1.6, -3], state)snap([1,1], bound(..., [1.6,-3]))[Math.round(1.6), -3] = [2, -3].
  8. Final state: coords = [[2, -3], [2, -1]] — both points at x = 2. getExponentialCoefficients returns undefined.

Impact

With both curve points at the same x-coordinate, the exponential coefficient calculation (denominator = x₁ − x₀ = 0) is undefined, corrupting graph state silently. Downstream scoring (PR4) and rendering (PR3) will receive incoherent state.

Fix

Add a same-x check immediately after computing the reflected point:

const reflectedPoint = boundAndSnapToGrid(
    [otherPoint[X], reflectedY],
    state,
);
if (reflectedPoint[X] === boundDestination[X]) {
    return state; // snapping caused x-collision; reject move
}
updatedCoords[otherIndex] = reflectedPoint;

Comment on lines +1935 to +1953
it("rejects the move when the asymptote would land between the curve points", () => {
// Arrange — curve points at y=3 and y=6; trying to move asymptote to y=4 (between them)
const state = generateExponentialGraphState({
coords: [
[0, 3],
[2, 6],
],
});

// Act
const updated = interactiveGraphReducer(
state,
actions.exponential.moveCenter([0, 4]),
);
invariant(updated.type === "exponential");

// Assert — y=4 < midpoint(4.5), so snaps to bottomMost - stepY = 3 - 1 = 2
expect(updated.asymptote).toBe(2);
});
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 test at line 1935 is titled "rejects the move when the asymptote would land between the curve points", but the move is not rejected — the asymptote snaps from 1 to 2. The assertion expect(updated.asymptote).toBe(2) and the inline comment ("snaps to bottomMost - stepY = 3 - 1 = 2") both confirm snap behavior. The title should read something like "snaps the asymptote to the nearest valid side when it would land between the curve points".

Extended reasoning...

What the bug is

The test title at line 1935 says "rejects the move when the asymptote would land between the curve points", but the tested behavior is a snap, not a rejection. The generateExponentialGraphState defaults set asymptote: 1. After calling actions.exponential.moveCenter([0, 4]), the test asserts expect(updated.asymptote).toBe(2). The asymptote changed — from 1 to 2 — so the move was not rejected.

The specific code path

In doMoveCenter for the "exponential" case (interactive-graph-reducer.ts), when the new asymptote position would fall between the two curve points, the reducer snaps to the nearest valid side:

newY = newY >= midpoint ? topMost + stepY : bottomMost - stepY;

With curve points at y=3 and y=6 and stepY=1, dragging to y=4 (below the midpoint of 4.5) snaps to bottomMost - stepY = 3 - 1 = 2. The inline comment in the test itself states exactly this: "y=4 < midpoint(4.5), so snaps to bottomMost - stepY = 3 - 1 = 2".

Why the title is wrong

"Rejects" means the state is unchanged — i.e., the asymptote would remain at its original value (1). But the assertion checks for 2, confirming the state did change. A developer reading a test failure for this test would expect the asymptote to be 1 (unchanged), but the assertion checks for 2, making the failure message misleading.

Impact

This is a test-only issue with no production code impact. However, CLAUDE.md specifies that "a failing test title should tell you which requirement broke without reading the test body." A developer debugging a regression in this test would be confused about whether the expected behavior is "reject" or "snap", slowing down diagnosis.

Step-by-step proof

  1. generateExponentialGraphState() creates state with asymptote: 1, coords: [[0,3],[2,6]], snapStep: [1,1].
  2. Action: actions.exponential.moveCenter([0, 4]) — attempting to set asymptote to y=4.
  3. newY = boundAndSnapToGrid([0,4], state)[Y] = 4.
  4. allAbove = (3>4 && 6>4) = false; allBelow = (3<4 && 6<4) = false — neither, so enters correction block.
  5. topMost=6, bottomMost=3, midpoint=4.5; since 4 < 4.5, newY = 3 - 1 = 2.
  6. newY (2) !== 3 && newY (2) !== 6 — passes the final guard.
  7. Result: asymptote = 2. The asymptote moved (1 → 2); the move was not rejected.

How to fix

Rename the test title to accurately describe the snap behavior, e.g.: "snaps the asymptote to the nearest valid side when it would land between the curve points".

@SonicScrewdriver SonicScrewdriver merged commit 85f9cd4 into main Mar 23, 2026
12 checks passed
@SonicScrewdriver SonicScrewdriver deleted the LEMS-3711/exponential-pr2 branch March 23, 2026 22:55
Comment on lines +750 to +757
newY = newY >= midpoint ? topMost + stepY : bottomMost - stepY;
newY = clamp(newY, yRange[0], yRange[1]);
}

// Final safety: asymptote must not land exactly on either point
if (newY === coords[0][Y] || newY === coords[1][Y]) {
return state;
}
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 snap correction in doMoveCenter for exponential graphs has two related bugs at lines 750-757: (1) topMost/bottomMost are taken directly from unsnapped state.coords, so adding/subtracting stepY to a float value (e.g. 2.3 - 1 = 1.3) produces a non-grid-aligned asymptote even when snapStep = 1; (2) when a curve point sits at yRange[1] (reachable via float startCoords), topMost + stepY overshoots the range, clamp returns it to topMost, and the final guard silently rejects the move, making it impossible to drag the asymptote above both points near the top boundary. Fix by re-snapping newY to the grid after the correction, and falling back to the opposite direction rather than silently rejecting when the preferred side is clamped onto a curve point.

Extended reasoning...

Bug 1: Non-grid-aligned asymptote from float startCoords

getExponentialCoords stores graph.startCoords.coords directly without snapping. A content author can supply startCoords: { coords: [[0, 2.3], [3, 4.7]], asymptote: 1 }, producing a valid but non-integer initial state. When the user drags the asymptote into the snap-correction branch of doMoveCenter, the code computes:

newY = newY >= midpoint ? topMost + stepY : bottomMost - stepY;
newY = clamp(newY, yRange[0], yRange[1]);

With bottomMost = 2.3 and stepY = 1, the corrected value is 2.3 - 1 = 1.3. The subsequent clamp only constrains to the graph range — it does not re-snap to the grid. The asymptote is stored as 1.3 despite snapStep = 1.

Step-by-step proof (bug 1)

  1. startCoords = { coords: [[0, 2.3], [3, 4.7]], asymptote: 1 }, snapStep=[1,1], range=[[-10,10],[-10,10]]
  2. State: coords=[[0,2.3],[3,4.7]], asymptote=1
  3. User drags asymptote to [0, 3]
  4. newY = boundAndSnapToGrid([0,3], state)[Y] = 3
  5. allAbove=(2.3>3 && 4.7>3)=false; allBelow=false — enters correction
  6. topMost=4.7, bottomMost=2.3, midpoint=3.5; 3 < 3.5 so newY = 2.3 - 1 = 1.3
  7. clamp(1.3, -10, 10) = 1.3 (no clamping)
  8. Final guard: 1.3 != 2.3 && 1.3 != 4.7 — passes
  9. Result: asymptote = 1.3 — not grid-aligned despite snapStep=1

Bug 2: Silent rejection when snap correction is clamped onto a curve point

When a curve point's Y value equals yRange[1] (reachable via float startCoords since getExponentialCoords does not bound them), topMost + stepY overshoots the range. clamp returns yRange[1] = topMost. The final guard then sees newY === coords[...][Y] and silently rejects the move. The user can move the asymptote below both points but cannot move it above them when they are near the top boundary.

Step-by-step proof (bug 2)

  1. State: coords=[[0,8],[2,10]], asymptote=0, snapStep=[1,1], range=[[-10,10],[-10,10]]
  2. User drags asymptote to [0, 9]
  3. newY = boundAndSnapToGrid([0,9], state)[Y] = 9
  4. allAbove=(8>9 && 10>9)=false; allBelow=false — enters correction
  5. topMost=10, bottomMost=8, midpoint=9; 9 >= 9 so newY = 10 + 1 = 11
  6. clamp(11, -10, 10) = 10
  7. Final guard: 10 === coords[1][Y]=10return state (silently rejected)

The curve point at y=10 is reachable because getExponentialCoords stores startCoords.coords without bounding to the graph range.

Why existing code does not prevent it

The clamp after the snap correction handles range overshoot but does not re-snap to the grid. The final guard prevents the asymptote from coinciding with a curve point, but fires in the clamped-overshoot case without trying the opposite direction as a fallback. Normal integer-valued coords are unaffected; both bugs only manifest when a content author supplies float Y values in startCoords.

Fix

After the snap correction, re-snap newY to the grid:

newY = snap([stepY], [0, newY])[Y];
newY = clamp(newY, yRange[0], yRange[1]);

And if the final guard would fire, try the opposite direction (bottomMost - stepY or topMost + stepY) rather than silently rejecting.

ivyolamit added a commit that referenced this pull request Mar 24, 2026
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-editor@30.0.0

### Major Changes

- [#3332](#3332)
[`604b3a6c25`](604b3a6)
Thanks [@benchristel](https://github.com/benchristel)! - The `options`
parameter of the `serialize` method of `EditorPage` and `Editor` has
been removed.


- [#3386](#3386)
[`7e76fbbc2f`](7e76fbb)
Thanks [@benchristel](https://github.com/benchristel)! - The `serialize`
methods of classes in `@khanacademy/perseus-editor` no longer use arrow
function syntax. Callers should not unbind them from the class instance.

Additionally, the `Editor` component no longer accepts a `replace` prop
(used for hints), and its serialize method no longer returns `replace`.
The `replace` prop was only used in `serialize`. Users of the `Editor`
component should manage hints' `replace` setting themselves.

### Minor Changes

- [#3395](#3395)
[`97223334ea`](9722333)
Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! -
Implementation of Editor support for Exponential Graph


- [#3352](#3352)
[`b681e00a4f`](b681e00)
Thanks [@handeyeco](https://github.com/handeyeco)! - Add editor support
for AbsoluteValue


- [#3348](#3348)
[`b1557c2a73`](b1557c2)
Thanks [@handeyeco](https://github.com/handeyeco)! - Add schema for
AbsoluteValue graph


- [#3345](#3345)
[`dde985f3b5`](dde985f)
Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add tangent type
definitions, this is the initial implementation for supporting Tangent
graph in Interactive Graph


- [#3358](#3358)
[`8c503171b1`](8c50317)
Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add tangent graph
option in the Interactive Graph Editor


- [#3376](#3376)
[`8aa0a77886`](8aa0a77)
Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! -
Creation of new Types, Schema, and Kmath utilities for Exponential Graph

### Patch Changes

- [#3396](#3396)
[`35fa9133db`](35fa913)
Thanks [@nishasy](https://github.com/nishasy)! - [Image] | (CX) | Add a
linter warning for images with no size


- [#3390](#3390)
[`d22c50dc2a`](d22c50d)
Thanks [@nishasy](https://github.com/nishasy)! - [Image] | (CX) | Make
the 125 character alt text warning less aggressive


- [#3372](#3372)
[`3cdb09813d`](3cdb098)
Thanks [@nishasy](https://github.com/nishasy)! - [Image] | (UX) |
Upscale Graphies within Explore Image Modal


- [#3391](#3391)
[`2f285ee161`](2f285ee)
Thanks [@nishasy](https://github.com/nishasy)! - [Image] | (CX) | Add
character counter to alt text field


- [#3374](#3374)
[`cd73c99ba3`](cd73c99)
Thanks [@ivyolamit](https://github.com/ivyolamit)! - Remove incorrect
usage of the feature flag setting in one of the test

- Updated dependencies
\[[`f18c0d9b6f`](f18c0d9),
[`a022e751d6`](a022e75),
[`35fa9133db`](35fa913),
[`54db3fd4bd`](54db3fd),
[`97223334ea`](9722333),
[`027a5edbda`](027a5ed),
[`ae0538d0a7`](ae0538d),
[`005e13d784`](005e13d),
[`3cdb09813d`](3cdb098),
[`afcff9f96f`](afcff9f),
[`75f184e5a7`](75f184e),
[`4b2a7c85db`](4b2a7c8),
[`5e1acd01f8`](5e1acd0),
[`b681e00a4f`](b681e00),
[`d99f1c0259`](d99f1c0),
[`54eee35d65`](54eee35),
[`b1557c2a73`](b1557c2),
[`dde985f3b5`](dde985f),
[`56e7dbe9a2`](56e7dbe),
[`85f9cd46fc`](85f9cd4),
[`8c503171b1`](8c50317),
[`3aca3dcdf4`](3aca3dc),
[`9f29bc7161`](9f29bc7),
[`7034844845`](7034844),
[`8aa0a77886`](8aa0a77),
[`003aca7612`](003aca7)]:
    -   @khanacademy/perseus-linter@4.9.0
    -   @khanacademy/perseus-score@8.4.0
    -   @khanacademy/perseus-core@23.7.0
    -   @khanacademy/perseus@76.1.0
    -   @khanacademy/kmath@2.3.0
    -   @khanacademy/keypad-context@3.2.40
    -   @khanacademy/math-input@26.4.10

## @khanacademy/kmath@2.3.0

### Minor Changes

- [#3351](#3351)
[`005e13d784`](005e13d)
Thanks [@handeyeco](https://github.com/handeyeco)! - Add scoring for
AbsoluteValue


- [#3347](#3347)
[`d99f1c0259`](d99f1c0)
Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add the tangent
math utilities to kmath for supporting Tangent graph in Interactive
Graph


- [#3376](#3376)
[`8aa0a77886`](8aa0a77)
Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! -
Creation of new Types, Schema, and Kmath utilities for Exponential Graph

### Patch Changes

- Updated dependencies
\[[`54db3fd4bd`](54db3fd),
[`ae0538d0a7`](ae0538d),
[`005e13d784`](005e13d),
[`b1557c2a73`](b1557c2),
[`dde985f3b5`](dde985f),
[`8aa0a77886`](8aa0a77)]:
    -   @khanacademy/perseus-core@23.7.0

## @khanacademy/perseus@76.1.0

### Minor Changes

- [#3350](#3350)
[`75f184e5a7`](75f184e)
Thanks [@handeyeco](https://github.com/handeyeco)! - Implement
AbsoluteValue rendering


- [#3354](#3354)
[`4b2a7c85db`](4b2a7c8)
Thanks [@ivyolamit](https://github.com/ivyolamit)! - Created the tangent
graph visual component, add Storybook coverage, SR strings, and equation
string for supporting Tangent graph in Interactive Graph


- [#3353](#3353)
[`5e1acd01f8`](5e1acd0)
Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add tangent graph
state management and reducer for supporting Tangent graph in Interactive
Graph


- [#3352](#3352)
[`b681e00a4f`](b681e00)
Thanks [@handeyeco](https://github.com/handeyeco)! - Add editor support
for AbsoluteValue


- [#3348](#3348)
[`b1557c2a73`](b1557c2)
Thanks [@handeyeco](https://github.com/handeyeco)! - Add schema for
AbsoluteValue graph


- [#3345](#3345)
[`dde985f3b5`](dde985f)
Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add tangent type
definitions, this is the initial implementation for supporting Tangent
graph in Interactive Graph


- [#3349](#3349)
[`56e7dbe9a2`](56e7dbe)
Thanks [@handeyeco](https://github.com/handeyeco)! - Add state
management for AbsoluteValue


- [#3377](#3377)
[`85f9cd46fc`](85f9cd4)
Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! -
Implementation of state management logic for new Exponential graph


- [#3358](#3358)
[`8c503171b1`](8c50317)
Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add tangent graph
option in the Interactive Graph Editor


- [#3393](#3393)
[`9f29bc7161`](9f29bc7)
Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! -
Rendering logic for new Exponential Graph


- [#3376](#3376)
[`8aa0a77886`](8aa0a77)
Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! -
Creation of new Types, Schema, and Kmath utilities for Exponential Graph

### Patch Changes

- [#3329](#3329)
[`027a5edbda`](027a5ed)
Thanks [@Myranae](https://github.com/Myranae)! - Fix image bug by
batching setState calls in setupGraphie


- [#3372](#3372)
[`3cdb09813d`](3cdb098)
Thanks [@nishasy](https://github.com/nishasy)! - [Image] | (UX) |
Upscale Graphies within Explore Image Modal


- [#3365](#3365)
[`afcff9f96f`](afcff9f)
Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Improve
ordering of Props type for `Renderer` component


- [#3367](#3367)
[`54eee35d65`](54eee35)
Thanks [@nishasy](https://github.com/nishasy)! - [Image] | (UX) | Show
image in explore modal even when size is undefined


- [#3407](#3407)
[`3aca3dcdf4`](3aca3dc)
Thanks [@Myranae](https://github.com/Myranae)! - Improve a11y with
graded group set


- [#3385](#3385)
[`003aca7612`](003aca7)
Thanks [@Myranae](https://github.com/Myranae)! - Small fix to prevent
pip duplication in Graded Group Sets

- Updated dependencies
\[[`f18c0d9b6f`](f18c0d9),
[`a022e751d6`](a022e75),
[`35fa9133db`](35fa913),
[`54db3fd4bd`](54db3fd),
[`97223334ea`](9722333),
[`ae0538d0a7`](ae0538d),
[`005e13d784`](005e13d),
[`d99f1c0259`](d99f1c0),
[`b1557c2a73`](b1557c2),
[`dde985f3b5`](dde985f),
[`7034844845`](7034844),
[`8aa0a77886`](8aa0a77)]:
    -   @khanacademy/perseus-linter@4.9.0
    -   @khanacademy/perseus-score@8.4.0
    -   @khanacademy/perseus-core@23.7.0
    -   @khanacademy/kmath@2.3.0
    -   @khanacademy/keypad-context@3.2.40
    -   @khanacademy/math-input@26.4.10

## @khanacademy/perseus-core@23.7.0

### Minor Changes

- [#3405](#3405)
[`54db3fd4bd`](54db3fd)
Thanks [@benchristel](https://github.com/benchristel)! -
`@khanacademy/perseus-core` now exports a
`removeOrphanedWidgetsFromPerseusItem` function, which removes
unreferenced widgets from a `PerseusItem`'s question and hints.


- [#3351](#3351)
[`005e13d784`](005e13d)
Thanks [@handeyeco](https://github.com/handeyeco)! - Add scoring for
AbsoluteValue


- [#3348](#3348)
[`b1557c2a73`](b1557c2)
Thanks [@handeyeco](https://github.com/handeyeco)! - Add schema for
AbsoluteValue graph


- [#3345](#3345)
[`dde985f3b5`](dde985f)
Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add tangent type
definitions, this is the initial implementation for supporting Tangent
graph in Interactive Graph


- [#3376](#3376)
[`8aa0a77886`](8aa0a77)
Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! -
Creation of new Types, Schema, and Kmath utilities for Exponential Graph

### Patch Changes

- [#3357](#3357)
[`ae0538d0a7`](ae0538d)
Thanks [@jeremywiebe](https://github.com/jeremywiebe)! - Improve code
documentation for all data-schema and user-input types

## @khanacademy/perseus-linter@4.9.0

### Minor Changes

- [#3381](#3381)
[`f18c0d9b6f`](f18c0d9)
Thanks [@anakaren-rojas](https://github.com/anakaren-rojas)! - Adds new
linters for parsed objects


- [#3395](#3395)
[`97223334ea`](9722333)
Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! -
Implementation of Editor support for Exponential Graph

### Patch Changes

- [#3396](#3396)
[`35fa9133db`](35fa913)
Thanks [@nishasy](https://github.com/nishasy)! - [Image] | (CX) | Add a
linter warning for images with no size

- Updated dependencies
\[[`54db3fd4bd`](54db3fd),
[`ae0538d0a7`](ae0538d),
[`005e13d784`](005e13d),
[`d99f1c0259`](d99f1c0),
[`b1557c2a73`](b1557c2),
[`dde985f3b5`](dde985f),
[`8aa0a77886`](8aa0a77)]:
    -   @khanacademy/perseus-core@23.7.0
    -   @khanacademy/kmath@2.3.0

## @khanacademy/perseus-score@8.4.0

### Minor Changes

- [#3356](#3356)
[`a022e751d6`](a022e75)
Thanks [@ivyolamit](https://github.com/ivyolamit)! - Add tangent graph
scoring to support the Tangent graph in Interactive Graph


- [#3351](#3351)
[`005e13d784`](005e13d)
Thanks [@handeyeco](https://github.com/handeyeco)! - Add scoring for
AbsoluteValue


- [#3394](#3394)
[`7034844845`](7034844)
Thanks [@SonicScrewdriver](https://github.com/SonicScrewdriver)! -
Implementation of new scoring logic for Exponential Graph

### Patch Changes

- Updated dependencies
\[[`54db3fd4bd`](54db3fd),
[`ae0538d0a7`](ae0538d),
[`005e13d784`](005e13d),
[`d99f1c0259`](d99f1c0),
[`b1557c2a73`](b1557c2),
[`dde985f3b5`](dde985f),
[`8aa0a77886`](8aa0a77)]:
    -   @khanacademy/perseus-core@23.7.0
    -   @khanacademy/kmath@2.3.0

## @khanacademy/keypad-context@3.2.40

### Patch Changes

- Updated dependencies
\[[`54db3fd4bd`](54db3fd),
[`ae0538d0a7`](ae0538d),
[`005e13d784`](005e13d),
[`b1557c2a73`](b1557c2),
[`dde985f3b5`](dde985f),
[`8aa0a77886`](8aa0a77)]:
    -   @khanacademy/perseus-core@23.7.0

## @khanacademy/math-input@26.4.10

### Patch Changes

- Updated dependencies
\[[`54db3fd4bd`](54db3fd),
[`ae0538d0a7`](ae0538d),
[`005e13d784`](005e13d),
[`b1557c2a73`](b1557c2),
[`dde985f3b5`](dde985f),
[`8aa0a77886`](8aa0a77)]:
    -   @khanacademy/perseus-core@23.7.0
    -   @khanacademy/keypad-context@3.2.40
catandthemachines pushed a commit that referenced this pull request Apr 1, 2026
### Summary:
_This PR was created with the help of AI, albeit with heavy oversight and review._

This is part of a series of PRs implementing the exponential graph type for
the Interactive Graph widget:

   PR1 – type definitions and schema
▶️ PR2 – state management (this PR)
   PR3 – rendering & accessibility
   PR4 – scoring
   PR5 – editor support

**Issue:** LEMS-3711

- Added state initialization, actions, and reducer logic for the exponential graph type

- Implements the full state management layer for exponential graphs. No rendering
  is wired up yet — the `"exponential"` case in `mafs-graph.tsx` still throws
  "Not implemented", so there is no visible change to users.

### Changes:

- **initialize-graph-state.ts** — `initializeGraphState` now handles `"exponential"`
  via a new `getExponentialCoords()` helper, which reads explicit coords and
  asymptote from the graph config, falls back to `startCoords` if set, and
  otherwise defaults to two normalized points above the x-axis with the asymptote
  at y=0.
- **interactive-graph-action.ts** — Registers `exponential: { movePoint, moveCenter }`
  in the `actions` namespace.
- **interactive-graph-reducer.ts** — Implements `doMovePoint` and `doMoveCenter`
  for exponential graphs:
  - `doMovePoint`: bounds and snaps the destination, prevents landing on the
    asymptote or sharing an x-value with the other point. If a point is dragged
    past the asymptote, the other point is reflected across it (mirrors the
    logarithm reducer, preserving the constraint that both points stay on the
    same side).
  - `doMoveCenter`: moves the asymptote vertically only. If the new y-value would
    land between the curve points, snaps to the nearest valid side. Also blocks
    the move if the asymptote would coincide exactly with either point.
- **interactive-graph-state.ts** — `getGradableGraph` extracts `coords` and
  `asymptote` from exponential state for scoring.
- **mafs-state-to-interactive-graph.ts** — Handles the `"exponential"` case,
  mapping state `coords` and `asymptote` back to the graph config.
- **interactive-graph-question-builder.ts** — Adds `withExponential()` builder
  method and `ExponentialGraphConfig` class. `asymptote` is a scalar `number`
  throughout, consistent with the schema from PR1.
- **interactive-graph.testdata.ts** — Adds `exponentialQuestion` test fixture.
- **reducer/initialize-graph-state.test.ts** — Tests for `getExponentialCoords`
  covering explicit coords, startCoords fallback, and default behavior.
- **reducer/interactive-graph-reducer.test.ts** — Tests for `doMovePoint` and
  `doMoveCenter`, including invalid moves, asymptote snapping, and point
  reflection across the asymptote.
- **packages/perseus/src/index.ts** — Exports `getExponentialCoords`.

### Test plan:
- [ ] `pnpm test packages/perseus` — all new reducer and initializer tests pass
- [ ] `pnpm tsc` — no new type errors
- [ ] `pnpm lint` — no lint errors
- [ ] No existing interactive graph tests regress

Author: SonicScrewdriver

Reviewers: claude[bot], ivyolamit

Required Reviewers:

Approved By: ivyolamit

Checks: ⌛ 1 check is pending, ✅ 10 checks were successful, ⏭️  1 check has been skipped

Pull Request URL: #3377
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants