Skip to content

[Interactive Graph] Add logarithm graph scoring#3424

Open
ivyolamit wants to merge 2 commits intoLEMS-3953/pr4-logarithm-graph-componentfrom
LEMS-3953/pr5-logarithm-scoring
Open

[Interactive Graph] Add logarithm graph scoring#3424
ivyolamit wants to merge 2 commits intoLEMS-3953/pr4-logarithm-graph-componentfrom
LEMS-3953/pr5-logarithm-scoring

Conversation

@ivyolamit
Copy link
Copy Markdown
Contributor

@ivyolamit ivyolamit commented Mar 27, 2026

Summary:

PR series to add logarithm graph support to the Interactive Graph widget:

  1. Add logarithm graph type definitions and data
  2. Add logarithm math utilities to kmath
  3. Add logarithm graph state management and reducer
  4. Add logarithm graph rendering, SR strings, and equation string
  5. ▶️ Add logarithm graph scoring
  6. Add logarithm graph option in the Interactive Graph Editor

Add logarithm graph scoring to support the Logarithm graph in Interactive Graph

  • Add logarithm scoring to score-interactive-graph.ts using direct coefficient comparison
  • No canonical normalization needed (same as exponential)

Details

Follows the exponential scoring pattern exactly. Computes {a, b, c} coefficients for both the user's answer and the rubric using getLogarithmCoefficients from kmath, then compares with approximateDeepEqual.

Scoring logic:

  • Returns invalid if guess coords or asymptote are missing, or if coefficient computation fails for either side
  • Returns earned: 1 if [a, b, c] coefficients approximately match
  • Returns earned: 0 otherwise

This means two different sets of control points that define the same logarithmic curve will score as correct — the comparison is on the mathematical function, not the specific points chosen.

Co-Authored by Claude Code (Opus)

Issue: LEMS-3953

Test plan:

  • pnpm tsc passes
  • pnpm knip passes
  • pnpm lint passes
  • pnpm prettier . --check passes
  • Scoring tests pass (44 total, 6 new logarithm tests):
  • Invalid: undefined guess, null coords, null asymptote
  • Correct: matching coefficients
  • Incorrect: different coefficients
  • Equivalent curves: different control points producing same y = ln(x) coefficients → correct

@ivyolamit ivyolamit self-assigned this Mar 27, 2026
@github-actions github-actions bot added item-splitting-change schema-change Attached to PRs when we detect Perseus Schema changes in it and removed schema-change Attached to PRs when we detect Perseus Schema changes in it labels Mar 27, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 27, 2026

🗄️ Schema Change: No Changes ✅

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 27, 2026

npm Snapshot: Published

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

Example:

pnpm add @khanacademy/perseus@PR3424

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

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

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

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 27, 2026

Size Change: +42 B (+0.01%)

Total Size: 496 kB

Filename Size Change
packages/perseus-score/dist/es/index.js 9.7 kB +42 B (+0.43%)
ℹ️ 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.35 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 12 kB
packages/perseus-core/dist/es/index.js 25.2 kB
packages/perseus-editor/dist/es/index.js 101 kB
packages/perseus-linter/dist/es/index.js 9.3 kB
packages/perseus-utils/dist/es/index.js 403 B
packages/perseus/dist/es/index.js 194 kB
packages/perseus/dist/es/strings.js 8.27 kB
packages/pure-markdown/dist/es/index.js 1.39 kB
packages/simple-markdown/dist/es/index.js 6.71 kB

compressed-size-action

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 27, 2026

🛠️ Item Splitting: No Changes ✅

@ivyolamit ivyolamit force-pushed the LEMS-3953/pr5-logarithm-scoring branch 2 times, most recently from 57320af to 7c67738 Compare March 27, 2026 19:09
@ivyolamit ivyolamit marked this pull request as ready for review March 27, 2026 19:11
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 and subscribe this PR to future pushes, or @claude review once for a one-time review.

Tip: disable this comment in your organization's Code Review settings.

@ivyolamit
Copy link
Copy Markdown
Contributor Author

@claude review once

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.

LGTM — straightforward addition that mirrors the exponential scoring pattern exactly, with solid test coverage.

Extended reasoning...

Overview

The PR adds logarithm graph scoring to score-interactive-graph.ts by importing getLogarithmCoefficients from kmath and inserting a new else if block following the existing exponential scoring block verbatim. A changeset file and 6 new test cases (invalid/correct/incorrect/equivalent-curve) are also included.

Security risks

None. This is pure scoring logic — no auth, no I/O, no user-controlled code paths beyond the already-gated coefficient computation.

Level of scrutiny

Low. The change is mechanical and self-contained: it adds ~30 lines of logic that are a direct copy of the adjacent exponential scoring block, only substituting getLogarithmCoefficients for getExponentialCoefficients. The pattern is already well-established and tested throughout the file.

Other factors

Test coverage is thorough for the new path (undefined guess, missing coords, missing asymptote, correct match, wrong match, and equivalent-curve normalization). Bundle size impact is negligible (+42 B). No CODEOWNERS concerns for this file.

@ivyolamit ivyolamit force-pushed the LEMS-3953/pr5-logarithm-scoring branch from 8976066 to 680fbf2 Compare March 28, 2026 00:42
Copy link
Copy Markdown
Contributor

@SonicScrewdriver SonicScrewdriver left a comment

Choose a reason for hiding this comment

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

Looks good!

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.

3 participants