Skip to content

[Interactive Graph] Add tangent graph scoring#3356

Open
ivyolamit wants to merge 2 commits intoLEMS-3955/add-tangent-graph-componentfrom
LEMS-3955/tangent-scoring
Open

[Interactive Graph] Add tangent graph scoring#3356
ivyolamit wants to merge 2 commits intoLEMS-3955/add-tangent-graph-componentfrom
LEMS-3955/tangent-scoring

Conversation

@ivyolamit
Copy link
Contributor

@ivyolamit ivyolamit commented Mar 13, 2026

Summary:

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

  1. Foundation — Add tangent graph type definitions and data schema
  2. Math layer — Add tangent math utilities to kmath
  3. State management — Reducer, actions, initialization, and test data
  4. Rendering — The tangent graph component, Storybook story, and AI utils
  5. ▶️ Scoring — Add tangent scoring to the scoring package
  6. Editor — Add tangent to answer type

This is the fifth PR in a series to add tangent graph support to the Interactive Graph widget (LEMS-3955). It adds the scoring layer — the final piece needed for tangent exercises to be fully functional (behind the feature flag).

Add tangent graph scoring to support the Tangent graph in Interactive Graph

  • Adds tangent scoring to scoreInteractiveGraph() using getTangentCoefficients and canonicalTangentCoefficients from kmath
  • Follows the sinusoid scoring pattern: extract coefficients from both guess and rubric, canonicalize, then compare
  • 6 new tests covering invalid input, correct/incorrect answers, equivalent curves, and negative amplitude
Implementation notes

Scoring follows the sinusoid pattern exactly. The tangent scoring block extracts coefficients from both the user's guess and the rubric's correct answer using getTangentCoefficients() (from kmath), canonicalizes both with canonicalTangentCoefficients(), and compares with approximateDeepEqual(). This handles equivalent curves that use different control points (e.g., shifted by a full period).

Uses kmath's canonicalTangentCoefficients, NOT the legacy grapher-util version. The kmath version (PR 2) only guarantees b > 0, which is mathematically correct for tangent. The legacy version guarantees both a > 0 and b > 0 using a mathematically incorrect phase shift. See PR 2 implementation notes for details.

References

Co-Authored by Claude Code (Opus)

Issue: LEMS-3955

Test plan:

  • pnpm tsc — no type errors
  • pnpm lint — no lint errors
  • pnpm prettier . --check — formatting clean
  • pnpm knip — no unused exports
  • Invalid input tests pass (undefined guess, missing coords)
  • Correct answer test passes
  • Incorrect answer test passes
  • Equivalent curves test passes (period-shifted control points)
  • Negative amplitude test passes

@ivyolamit ivyolamit self-assigned this Mar 13, 2026
@github-actions
Copy link
Contributor

github-actions bot commented Mar 13, 2026

🗄️ Schema Change: No Changes ✅

@github-actions
Copy link
Contributor

github-actions bot commented Mar 13, 2026

🛠️ Item Splitting: No Changes ✅

@github-actions
Copy link
Contributor

github-actions bot commented Mar 13, 2026

Size Change: +43 B (+0.01%)

Total Size: 487 kB

Filename Size Change
packages/perseus-score/dist/es/index.js 9.3 kB +43 B (+0.46%)
ℹ️ View Unchanged
Filename Size
packages/kas/dist/es/index.js 20.8 kB
packages/keypad-context/dist/es/index.js 1 kB
packages/kmath/dist/es/index.js 6.03 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 100 kB
packages/perseus-linter/dist/es/index.js 8.82 kB
packages/perseus-utils/dist/es/index.js 403 B
packages/perseus/dist/es/index.js 188 kB
packages/perseus/dist/es/strings.js 7.66 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
Contributor

npm Snapshot: Published

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

Example:

pnpm add @khanacademy/perseus@PR3356

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

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

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

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

@ivyolamit ivyolamit marked this pull request as ready for review March 13, 2026 23:45
@ivyolamit
Copy link
Contributor Author

Summary of the tangent implementation

PR Scope Note
1 Foundation — Add tangent graph type definitions and data schema #3345 for review
2 Math layer — Add tangent math utilities to kmath #3347 for review
3 State management — Reducer, actions, initialization, and test data #3353 for review
4 Rendering — The tangent graph component, Storybook story, and AI utils #3354 base PR (for review)
5 Scoring — Add tangent scoring to the scoring package #3356 this PR
6 Editor — Add tangent to answer type 🔜

Copy link

@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 of tangent scoring that mirrors the existing sinusoid pattern exactly.

Extended reasoning...

Overview

This PR adds tangent graph scoring to scoreInteractiveGraph() in the perseus-score package. It touches 3 files: a changeset, the scoring implementation, and tests. The implementation adds ~26 lines of scoring logic (a new else if branch) and ~140 lines of tests (6 test cases).

Security risks

None. This is pure scoring logic that compares mathematical coefficients — no user-facing I/O, no network calls, no auth, no data persistence.

Level of scrutiny

Low scrutiny warranted. The tangent scoring block is a near-verbatim copy of the sinusoid scoring block, substituting getTangentCoefficients / canonicalTangentCoefficients for the sinusoid equivalents. The pattern is well-established in this codebase. The change is behind a feature flag per the PR description.

Other factors

  • 6 tests cover the key cases: undefined guess, missing coords, correct answer, incorrect answer, equivalent curves (period-shifted), and negative amplitude
  • No outstanding reviewer comments or prior reviews
  • No bugs found by the bug hunting system
  • Bundle size impact is negligible (+43 B)
  • The mathematical correctness of getTangentCoefficients and canonicalTangentCoefficients is the responsibility of the kmath package (separate PRs in this series), not this scoring layer

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.

1 participant