Skip to content

[Interactive Graph] Update tangent notes for AI shared context #3333

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ivyolamit merged 3 commits into from
Mar 23, 2026
Merged

[Interactive Graph] Update tangent notes for AI shared context #3333

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4d243d9
[LEMS-3937/update-tangent-notes] docs(changeset):
ivyolamit Mar 12, 2026
e045175
[LEMS-3937/update-tangent-notes] [Interactive Graph] Update tangent n…
ivyolamit Mar 12, 2026
7f54f17
[LEMS-3937/update-tangent-notes] updated based on claude review comment
ivyolamit Mar 12, 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
2 changes: 2 additions & 0 deletions .changeset/khaki-terms-brush.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
---
23 changes: 14 additions & 9 deletions packages/perseus/src/widgets/interactive-graphs/__docs__/notes/tangent.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ allowing content creators to define tangent function exercises using two movable
- No vertical lines are drawn across asymptotes (discontinuities are handled correctly)
- Keyboard navigation works on both control points (arrow keys move the point by snap step)
- If both points share the same x-coordinate, the move is rejected gracefully (no crash, no invalid state)
- The graph is scorable — the correct answer is compared using canonical coefficient normalization (amplitude > 0, angular frequency > 0, phase in [0, π))
- The graph is scorable — the correct answer is compared using canonical coefficient normalization (angular frequency > 0, phase in [0, π))
- Screen reader announces the graph label and point positions correctly
- The widget renders correctly on mobile

Expand All @@ -47,7 +47,7 @@ allowing content creators to define tangent function exercises using two movable

The Grapher widget already has a tangent graph type that served as the mathematical reference:

- `packages/perseus-core/src/utils/grapher-util.ts` — Tangent coefficient computation (`getTangentCoefficients`) and equation string generation. Uses the same `f(x) = a * tan(b*x - c) + d` model. Also contains the `"sin("` → `"tan("` label bug (see [Related: Grapher Widget Bug](#related-grapher-widget-bug)).
- `packages/perseus-core/src/utils/grapher-util.ts` — Tangent coefficient computation (`getTangentCoefficients`), equation string generation, and its own copy of `canonicalTangentCoefficients()` (kept in sync with the kmath version). Uses the same `f(x) = a * tan(b*x - c) + d` model. Also contains the `"sin("` → `"tan("` label bug (see [Related: Grapher Widget Bug](#related-grapher-widget-bug)).
Copy link
Copy Markdown

@claude claude Bot Mar 12, 2026

Choose a reason for hiding this comment

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

🟡 The claim that grapher-util.ts's canonicalTangentCoefficients() is "kept in sync with the kmath version" (line 50) is inaccurate. The grapher-util.ts version (lines 58-91) guarantees both a > 0 and b > 0 using a phase += period/2 step, while this same PR's documentation elsewhere correctly states the kmath version can only guarantee b > 0 using the odd function identity. Consider rewording to note that the grapher-util.ts copy uses a different (and mathematically questionable) normalization strategy.

Extended reasoning...

What the bug is

Line 50 of the updated tangent.md introduces the phrase: grapher-util.ts has "its own copy of canonicalTangentCoefficients() (kept in sync with the kmath version)". This claim is internally inconsistent with the rest of the documentation added by this same PR.

The inconsistency in detail

The grapher-util.ts canonicalTangentCoefficients() (lines 58-91) implements a two-step normalization:

  1. Guarantee a > 0: flips signs of amplitude, angularFrequency, and phase
  2. Guarantee b > 0: flips angularFrequency and phase, then does phase += period/2

Meanwhile, the PR's own documentation at lines 113-116 describes the kmath version as using a fundamentally different approach:

  • Guarantee b > 0 using the odd function identity: a * tan(-|b|x - c) = (-a) * tan(|b|x - (-c)), which flips signs of a and c
  • Explicitly notes: "unlike sine... tangent has no such half-period identity, so only b > 0 can be guaranteed"

These are different algorithms with different guarantees (a > 0 AND b > 0 vs only b > 0).

Why the grapher-util.ts approach is mathematically questionable

The grapher-util.ts version uses phase += period/2 (i.e., phase += π/2) to guarantee b > 0. This step implicitly relies on tan(x + π/2) = -tan(x), but that identity is incorrect: tan(x + π/2) = -cot(x), not -tan(x). The sine version works because sin(x + π) = -sin(x) is a valid identity. The tangent version appears to be a copy-paste from the sine normalization with only the period changed from 2π to π, without accounting for the different trigonometric identities.

Step-by-step proof of the discrepancy

Consider f(x) = -1 * tan(-2x - 1) + 0 (a = -1, b = -2, c = 1, d = 0).

grapher-util.ts algorithm:

  1. a < 0, so flip: a = 1, b = 2, c = -1
  2. b > 0 already, skip step 2
  3. Normalize c: c = -1, add π → c ≈ 2.14
  4. Result: tan(2x - 2.14) with a = 1, b = 2

kmath algorithm (as documented):

  1. b < 0, so apply odd identity: a = -(-1) = 1, b = 2, c = -(-1) = -1 (wait, let me redo: flips signs of a and c)
    Actually: a * tan(-|b|x - c) = (-a) * tan(|b|x + c), so a becomes 1, c becomes -1, b becomes 2
  2. Normalize c: c = -1, add π → c ≈ 2.14
  3. Result: 1 * tan(2x - 2.14) with a = 1, b = 2

In this case they agree, but the key difference is that the grapher-util.ts version always guarantees a > 0 as an independent first step, while the kmath version does not have this guarantee. For cases where a < 0 and b > 0, the grapher-util.ts version will flip a positive while also flipping b negative, then use the questionable phase shift to fix b. The kmath version would leave a negative since b is already positive.

Impact

This is a documentation-only issue in an AI shared context notes file. The "kept in sync" claim could mislead developers or AI assistants into thinking the two implementations are interchangeable or identical, when they are not. Since this PR is specifically updating these notes for accuracy, the claim should be corrected.

Suggested fix

Replace "kept in sync with the kmath version" with something like "uses a different normalization algorithm that guarantees both a > 0 and b > 0, unlike the kmath version which only guarantees b > 0".

- `packages/perseus-core/src/utils/grapher-types.ts` — `TangentPlotDefaults` type with default coords and asymptote config
- `packages/perseus-core/src/data-schema.ts` — Existing `PerseusGrapherWidgetOptions` already includes `"tangent"` as a valid plot type
- `packages/perseus-score/src/widgets/grapher/score-grapher.ts` — Grapher scoring uses `coefficients.getTangentCoefficients()` and `geometry.canonicalTangentCoefficients()` for comparison
Expand All @@ -61,7 +61,7 @@ The sinusoid graph type was the primary pattern reference for the tangent implem
- `packages/perseus/src/widgets/interactive-graphs/reducer/initialize-graph-state.ts` — Sinusoid state initialization pattern (default coords, snap step). Tangent follows the same approach.
- `packages/perseus/src/widgets/interactive-graphs/types.ts` — `SinusoidGraphState` type. `TangentGraphState` follows the same shape.
- `packages/perseus-score/src/widgets/interactive-graph/score-interactive-graph.ts` — Sinusoid scoring block (canonical coefficient comparison with `approximateDeepEqual`). Tangent scoring is modeled after this.
- `packages/kmath/src/geometry.ts` — `canonicalSineCoefficients()`. The tangent equivalent normalizes with period `π` instead of `2π`.
- `packages/kmath/src/geometry.ts` — `canonicalSineCoefficients()`. The tangent equivalent normalizes with period `π` instead of `2π`, and can only guarantee `b > 0` (not both `a > 0` and `b > 0` like sine).
- `packages/kmath/src/coefficients.ts` — `getSinusoidCoefficients()`. `getTangentCoefficients()` was added alongside it.

### Mafs Graphing Library
Expand All @@ -83,7 +83,9 @@ Where:
- `c` = phase = `p1[x] * b`
- `d` = vertical offset = `p1[y]`

Two movable points define the curve (same pattern as sinusoid).
Two movable points define the curve (same pattern as sinusoid):
- `p1` is the inflection point (where `tan = 0`, i.e. the curve crosses through its vertical offset)
- `p2` is a quarter-period away and determines the amplitude and period

### Key Differences from Sinusoid

Expand All @@ -97,7 +99,7 @@ The implementation follows the same patterns established by the sinusoid graph t

### Rendering (`tangent.tsx`)

1. Compute coefficients from the two movable point coordinates
1. Compute coefficients from the two movable point coordinates (delegates to `@khanacademy/kmath`'s `getTangentCoefficients()` to keep the formula in one place)
2. Determine asymptote positions within the visible x-range
3. Split the curve into segments between asymptotes
4. Render each segment as a separate `<Plot.OfX>` with its own domain
Expand All @@ -106,15 +108,15 @@ The implementation follows the same patterns established by the sinusoid graph t
### Scoring (`score-interactive-graph.ts`)

1. Extract tangent coefficients from both user and rubric coordinates
2. Normalize to canonical form (ensure `a > 0`, `b > 0`, phase minimized)
2. Normalize to canonical form (ensure `b > 0`, phase minimized)
3. Use `approximateDeepEqual` to compare canonical coefficients

### Canonical Form Normalization (`geometry.ts`)

Ensures equivalent curves compare as equal:
- Guarantee `a > 0` (flip sign of `b` and `c` if needed)
- Guarantee `b > 0` (flip sign of `c` and shift phase by `period/2`)
- Guarantee `b > 0` using the odd function identity: `a * tan(-|b|x - c) = (-a) * tan(|b|x - (-c))`, which flips the signs of `a` and `c`
- Normalize `c` to smallest positive value within period `π`
- Note: unlike sine (where `sin(x + π) = -sin(x)` allows guaranteeing both `a > 0` and `b > 0`), tangent has no such half-period identity, so only `b > 0` can be guaranteed

### Constraints

Expand Down Expand Up @@ -169,7 +171,8 @@ Once Mafs fixes path generation to use `M` (moveTo) after non-finite gaps instea

### Modified files
- `packages/kmath/src/geometry.ts` — `TangentCoefficient` type, `canonicalTangentCoefficients()`
- `packages/kmath/src/coefficients.ts` — `getTangentCoefficients()`
- `packages/kmath/src/coefficients.ts` — `getTangentCoefficients()`, `NamedTangentCoefficient` type
- `packages/kmath/src/index.ts` — Re-exports `NamedTangentCoefficient`
- `packages/perseus-core/src/data-schema.ts` — `PerseusGraphTypeTangent`, `TangentGraphCorrect`
- `packages/perseus-core/.../interactive-graph-widget.ts` — Parser for tangent type
- `packages/perseus-score/.../score-interactive-graph.ts` — Tangent scoring logic
Expand Down Expand Up @@ -208,3 +211,5 @@ in the equation label. This was fixed in a separate PR on branch

5. **Canonical form for scoring** — Normalization ensures equivalent tangent curves
(which can be expressed with different coefficient signs) are scored correctly.
Unlike sine, tangent has no half-period phase shift identity (`sin(x + π) = -sin(x)`),
so the canonical form can only guarantee `b > 0`, not both `a > 0` and `b > 0`.
Loading