[Interactive Graph] Add logarithm graph state management and reducer #3422
Claude / Claude Code Review
completed
Mar 28, 2026 in 32m 21s
Code review found 3 potential issues
Found 5 candidates, confirmed 3. See review comments for details.
Details
| Severity | Count |
|---|---|
| 🔴 Important | 0 |
| 🟡 Nit | 2 |
| 🟣 Pre-existing | 1 |
| Severity | File:Line | Issue |
|---|---|---|
| 🟡 Nit | packages/perseus/src/widgets/interactive-graphs/reducer/initialize-graph-state.ts:521-524 |
getLogarithmCoords missing export keyword |
| 🟡 Nit | packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-reducer.ts:894-897 |
Dead code: final safety check in logarithm doMoveCenter is unreachable |
| 🟣 Pre-existing | packages/perseus/src/widgets/interactive-graphs/reducer/interactive-graph-reducer.ts:591-601 |
Exponential cross-asymptote reflection missing asymptote-Y guard |
Annotations
claude / Claude Code Review
getLogarithmCoords missing export keyword
The `getLogarithmCoords` function in `initialize-graph-state.ts` is missing the `export` keyword, unlike its counterpart `getExponentialCoords` (line 490) which is exported. The design doc (logarithm.md:307) explicitly lists exporting `getLogarithmCoords` as a PR 3 deliverable, and PR 6 (editor support) will need to import it exactly the way the editor currently imports `getExponentialCoords`.
claude / Claude Code Review
Dead code: final safety check in logarithm doMoveCenter is unreachable
The final safety check at lines 894-897 (`if (newX === coords[0][X] || newX === coords[1][X]) return state`) is logically unreachable dead code introduced by this PR. Both code paths that reach it guarantee strict inequality between `newX` and each coord's X value, so this guard can never fire. The identical pattern exists pre-existing in the exponential case at lines 849-852.
claude / Claude Code Review
Exponential cross-asymptote reflection missing asymptote-Y guard
This is a pre-existing issue in the exponential `doMovePoint` cross-asymptote reflection block: after calling `boundAndSnapToGrid` on the reflected point, there is no check that `updatedCoords[otherIndex][Y] === asymptoteY`. If `otherPoint[Y]` is non-grid-aligned (possible via `startCoords`, which are stored verbatim) and within `snapStep/2` of the asymptote, the reflected Y snaps to exactly `asymptoteY`, committing an invalid state with a curve point on the horizontal asymptote. The logarithm c
Loading