[LEMS-2190] Move coord reverse logic into scoring logic

[SonicScrewdriver]

Summary:

This PR is part of the Interactive Graph project.

It contains the work required to move the coordinate reversal logic from the getGradeableGraph function and move it directly into our scoring logic. This logic was initially left in getGradeableGraph while we waited for the legacy Angle graph to no longer be in use.

The reason for this was that there were 2 conflicting bugs in the Angle graph previously:

1. Bug 1 allowed graphs to be reflexive if the bottom coordinate was moved, even if allowReflexiveAngles was false.
2. Bug 2 failed to properly reverse the coordinates in the same situation, resulting in the answer being marked as incorrect.

Unfortunately, the combination of these bugs meant that we we needed to keep the scoring logic unchanged until the feature flags for the new Angle graph were removed.

Issue: LEMS-2190

Test plan:

• moved tests to new location
• tests pass

[github-actions]

npm Snapshot: Published

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

Example:

yarn add @khanacademy/perseus@PR2217

If you are working in Khan Academy's webapp, you can run:

./dev/tools/bump_perseus_version.sh -t PR2217

[github-actions]

Size Change: -14 B (0%)

Total Size: 1.48 MB

Filename	Size	Change
packages/perseus-score/dist/es/index.js	115 kB	+214 B (+0.19%)
packages/perseus/dist/es/index.js	379 kB	-228 B (-0.06%)

ℹ️ View Unchanged

Filename	Size
packages/kas/dist/es/index.js	39 kB
packages/keypad-context/dist/es/index.js	760 B
packages/kmath/dist/es/index.js	86.9 kB
packages/math-input/dist/es/index.js	77.6 kB
packages/math-input/dist/es/strings.js	1.79 kB
packages/perseus-core/dist/es/index.js	46.3 kB
packages/perseus-editor/dist/es/index.js	688 kB
packages/perseus-linter/dist/es/index.js	22.2 kB
packages/perseus/dist/es/strings.js	6.06 kB
packages/pure-markdown/dist/es/index.js	3.66 kB
packages/simple-markdown/dist/es/index.js	12.5 kB

_{compressed-size-action}

[SonicScrewdriver]

I think this might have been an oversight, but do please let me know if we are intentionally commandeering the match setting for all Angle graphs to "congruent".

If we look at PerseusGraphTypeAngle, the match property is optional, and indicates that if it is not provided then we default to exact matching. There's some logic for handling this in score-interactive-graph.ts. It is possible that we may have introduced a bug for some Angle Graphs by hardcoding all answer checking to the "congruent" path.

PerseusGraphTypeAngle

    // How to match the answer. If missing, defaults to exact matching.
    match?: "congruent";

Scoring Logic

    if (rubric.correct.match === "congruent") {
                const angles = _.map([guess, correct], function (coords) {
                    if (!coords) {
                        return false;
                    }
                    const angle = getClockwiseAngle(coords, allowReflexAngles);
                    return angle;
                });
                // @ts-expect-error - TS2556 - A spread argument must either have a tuple type or be passed to a rest parameter.
                match = approximateEqual(...angles);
            } else {
                /* exact */
                match =
                    approximateDeepEqual(guess[1], correct[1]) &&
                    collinear(correct[1], correct[0], guess[0]) &&
                    collinear(correct[1], correct[2], guess[2]);
            }

If we do want to only match for "congruent", then we might want to remove this property altogether along with the additional logic for handling these two cases.

[nishasy]

I think match: "congruent" is pretty odd and confusing. If there's a match at all, it's always "congruent"? I feel like this should be a boolean...

In any case, I think making it optional to match the schema makes sense here. Also it doesn't make sense for it to not be optional, because then match is always there, and it's always "congruent."

[SonicScrewdriver]

There's really no possible way for coords to ever be less than 3 coordinates, due to how the graphs are set up.

I will make another ticket for setting a stricter type for the Angle Graph Coords, as it will require updating several tests that have only partially mocked data.

[nishasy]

Could you make the ticket now and update this comment to start with a TODO(LEMS-XXXX)?

[nishasy]

I like it!

[nishasy]

I think match: "congruent" is pretty odd and confusing. If there's a match at all, it's always "congruent"? I feel like this should be a boolean...

In any case, I think making it optional to match the schema makes sense here. Also it doesn't make sense for it to not be optional, because then match is always there, and it's always "congruent."

[nishasy]

Could you make the ticket now and update this comment to start with a TODO(LEMS-XXXX)?

[nishasy]

🎉

[nishasy]

Thank you for adding this test!