[Interactive Graph] Add tangent graph scoring#3356
[Interactive Graph] Add tangent graph scoring#3356ivyolamit wants to merge 2 commits intoLEMS-3955/add-tangent-graph-componentfrom
Conversation
…g to support the Tangent graph in Interactive Graph
🗄️ Schema Change: No Changes ✅ |
🛠️ Item Splitting: No Changes ✅ |
|
Size Change: +43 B (+0.01%) Total Size: 487 kB
ℹ️ View Unchanged
|
npm Snapshot: PublishedGood news!! We've packaged up the latest commit from this PR (355ee58) and published it to npm. You Example: pnpm add @khanacademy/perseus@PR3356If you are working in Khan Academy's frontend, you can run the below command. ./dev/tools/bump_perseus_version.ts -t PR3356If you are working in Khan Academy's webapp, you can run the below command. ./dev/tools/bump_perseus_version.js -t PR3356 |
|
Summary of the tangent implementation
|
There was a problem hiding this comment.
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
getTangentCoefficientsandcanonicalTangentCoefficientsis the responsibility of the kmath package (separate PRs in this series), not this scoring layer
Summary:
PR series to add tangent graph support to the Interactive Graph widget:
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
scoreInteractiveGraph()usinggetTangentCoefficientsandcanonicalTangentCoefficientsfrom kmathImplementation 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 withcanonicalTangentCoefficients(), and compares withapproximateDeepEqual(). 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 guaranteesb > 0, which is mathematically correct for tangent. The legacy version guarantees botha > 0andb > 0using 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 errorspnpm lint— no lint errorspnpm prettier . --check— formatting cleanpnpm knip— no unused exports