Bug fix: Fix Multiselect + Randomization only show correct for proper answers

[SonicScrewdriver]

Summary:

This quick bug-fix temporarily reverts a recent one-line change to fix a P1 bug for the Radio widget.

Bug History:

The issue we were facing was that additional answers were showing as "correct" upon score even though they were incorrect.

This bug is largely due to logic in the radio-component.tsx file on line 423 that uses the rubric to set the correct answer if it is not provided — as is often the case with our older content.

correct: choice.correct === undefined ? !!reviewChoice && !!reviewChoice.correct : choice.correct

The problem with this logic is that our choices have already been shuffled, but our rubric has not. As a result of this index mismatch, any questions that have turned on both Multi Selection AND Randomization could result in an additional answer or two being visually styled as if it is correct.

The bug has always existed, it was merely uncovered after a minor change from the Server Side Scoring project. Previously, we were never hitting the first condition of the ternary as radio.ts was explicitly setting choice.correct to a boolean value at an earlier stage. We're now hitting this condition after Server Side Scoring removed that line.

Temporary Solution:

This PR adds the removed line back to radio.ts so that we can solve the bug as quickly as possible — while we work with the Server Side Scoring team to come up with a better long-term solution.

One alternative proposal to this current PR would be to change the logic from radio-component.tsx (outlined above) to:

correct: Boolean(choice.correct)

This change is effectively the same as the one proposed this PR. As we were unsure if Server Side Scoring might require the reviewChoice logic in the future, we've opted for a simple revision. We're happy to adjust our approach however!

Issue: LEMS-2909

Test plan:

• manual testing
• tests pass

[github-actions]

Size Change: +9 B (0%)

Total Size: 735 kB

Filename	Size	Change
packages/perseus/dist/es/index.js	369 kB	+9 B (0%)

ℹ️ View Unchanged

Filename	Size
packages/kas/dist/es/index.js	39.7 kB
packages/keypad-context/dist/es/index.js	760 B
packages/kmath/dist/es/index.js	11.1 kB
packages/math-input/dist/es/index.js	77.5 kB
packages/math-input/dist/es/strings.js	1.79 kB
packages/perseus-core/dist/es/index.js	31.9 kB
packages/perseus-editor/dist/es/index.js	136 kB
packages/perseus-linter/dist/es/index.js	22.8 kB
packages/perseus-score/dist/es/index.js	20.6 kB
packages/perseus/dist/es/strings.js	6.73 kB
packages/pure-markdown/dist/es/index.js	4.14 kB
packages/simple-markdown/dist/es/index.js	13.1 kB

_{compressed-size-action}

[github-actions]

npm Snapshot: Published

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

Example:

pnpm add @khanacademy/perseus@PR2307

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

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

[handeyeco]

I think this is fine to ship as-is if we can patch a confusing bug for learners.

Just some thoughts though:

(1) Thank you for looking into this. 🫶 I hope this is just me being reckless and not an indication of how temperamental Radio is going to be during a refactor. 📻 I'm also surprised it took so long for us to find this issue! Anyway, sorry for the bug and thank you for the fix.

(2) I don't feel super comfortable approving bugfixes without tests. I know there's an urgency here, but I think that test coverage should be considered part of a bugfix. The issue isn't that I made this change, the issue is we didn't have enough coverage to reveal that I introduced a regression.

I'm mostly saying that in the abstract. Obviously I messed up, but I think it's a good way of thinking when anyone introduces a regression. The problem isn't the code change, it's the lack of coverage; so the solution is more coverage.

(3) This feels conceptually (or maybe just semantically) wrong to me and that has nothing to do with your change but with the original logic. Why are they trying to force correctness if it's possibly undefined? This is even more of a problem in the SSS world, this line of code is going to mark every choice as "incorrect" when they have public widget options and only mark things correctly when they have full widget options.

I don't think this is really an issue though, I just worry it's going to be confusing for devs trying to debug in the future: they're going to console log render props pre-scoring and all the answers are going to have correct: false. Really what I think we need to do is to properly handle boolean | undefined rather than try to force it into a boolean, then do a conversion when we actually want to do something with the data (ie const showCorrectness: boolean = Boolean(answer.correct)).

[handeyeco]

This doesn't revert SSS for Radio

Suggested change

	Revert SSS temporarily for Radio to ensure Multiselect + Random answer correctness is accurate
	Bugfix: Multiselect + Random in Radio sometimes displays items as correct erroneously.

[handeyeco]

I think it would be beneficial to have tests. I didn't trigger any failures when making this change, so there must be a gap in our coverage. Nothing's stopping me from making this change again in two weeks when I forget any of this ever happened.

[SonicScrewdriver]

We want to implement some better tests that will take a tiny bit more time than the quick bug fix. Given this is blocking users, we're going to land this now and then add the tests later today with LEMS-2931