Sometimes fraction input doesn't let you submit

[BenHenning]

No description provided.

[nikitamarysolomanpvt]

@BenHenning Could you please suggest a scenario.

[BenHenning]

Ah, Sean and I noticed that a few times when you try submitting a fraction the submit button doesn't do anything, and you stay stuck on the state. I noticed this last week, but I haven't yet consistently reproed it to fix it. I was going to investigate--do you have any ideas what might be causing it?

[seanlip]

On the card in the second Fractions lesson where it showed a 3/4 circle (red-shaded, with different sized pieces), I typed 3/2 to start with, got "this is wrong", and then typed some other fraction. It started hanging at that point, no matter what fraction I typed.

[BenHenning]

FWIW I believe you need to fully exit the exploration and re-enter to remediate.

[BenHenning]

Just caught this for one scenario: I tried entering '2' and it failed:

2019-11-19 23:50:13.886 20588-20588/org.oppia.app E/StateFragment: Failed to retrieve answer outcome
    java.lang.IllegalStateException: Failed when classifying answer # org.oppia.app.model.InteractionObject@7485258e
    fraction {
      denominator: 1
      whole_number: 2
    } for interaction FractionInput
        at org.oppia.domain.classify.AnswerClassificationController.classifyAnswer(AnswerClassificationController.kt:51)
        at org.oppia.domain.classify.AnswerClassificationController.classify$domain_debug(AnswerClassificationController.kt:29)
        at org.oppia.domain.exploration.ExplorationProgressController.submitAnswer(ExplorationProgressController.kt:130)
        at org.oppia.app.player.state.StateFragmentPresenter.handleSubmitAnswer(StateFragmentPresenter.kt:484)
        at org.oppia.app.player.state.StateFragmentPresenter.onSubmitButtonClicked(StateFragmentPresenter.kt:475)
        at org.oppia.app.player.state.itemviewmodel.StateNavigationButtonViewModel.triggerContinuationNavigationButtonCallback(StateNavigationButtonViewModel.kt:81)
        at org.oppia.app.databinding.StateButtonItemBindingImpl._internalCallbackOnClick(StateButtonItemBindingImpl.java:439)
        at org.oppia.app.generated.callback.OnClickListener.onClick(OnClickListener.java:11)
        at android.view.View.performClick(View.java:6597)
        at android.view.View.performClickInternal(View.java:6574)
        at android.view.View.access$3100(View.java:778)
        at android.view.View$PerformClick.run(View.java:25885)
        at android.os.Handler.handleCallback(Handler.java:873)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at android.os.Looper.loop(Looper.java:193)
        at android.app.ActivityThread.main(ActivityThread.java:6669)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:493)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:858)
     Caused by: java.lang.IllegalStateException: Expected input value to be of type NON_NEGATIVE_INT not REAL
        at org.oppia.domain.classify.rules.GenericRuleClassifier.retrieveInputObject(GenericRuleClassifier.kt:36)
        at org.oppia.domain.classify.rules.GenericRuleClassifier.matches(GenericRuleClassifier.kt:25)
        at org.oppia.domain.classify.AnswerClassificationController.classifyAnswer(AnswerClassificationController.kt:46)

It didn't end up permanently broken, though, so I'm not sure this is the same problem. I'm also confused why the InteractionObject type is being mixed up...it should always be a fraction.

[BenHenning]

Aha! I then put in a wrong fraction and it parsed, but now every input after appears broken. Investigating why it's permanently borked.

[BenHenning]

I'm getting closer on why it's failing, but I have no clue why it permanently breaks.

[BenHenning]

Ah, I think it's because certain answers trigger a bad rule to be evaluated. I suspect if I put the right answer in it may fix itself. I'm not sure why it didn't fix for you, @seanlip. Will check to see if that remediates momentarily.

[BenHenning]

Okay, we're hitting a HasDenominatorEqualTo and the x value being provided is a double, but it's supposed to be an int per https://github.com/oppia/oppia/blob/11c73a781ecdc221947fcac5bd5838909d14bdf7/extensions/interactions/rule_templates.json#L61 which is also what the classifier is expecting, so it's throwing an exception and failing.

[BenHenning]

Assuming this is a parsing issue, but not sure yet.

[BenHenning]

Confirming submitting the correct answer seems to fix the state. Not sure what we hit earlier Sean, but I'm pretty sure this is the same issue.

[BenHenning]

Hmm

oppia-android/domain/src/main/java/org/oppia/domain/util/StateRetriever.kt

Line 184 in 55f959e

"HasDenominatorEqualTo", "HasNumeratorEqualTo" -> ruleSpecBuilder.putInput(

seems to be wrong, and likely culprit.

[seanlip]

FYI I never submitted the correct answer earlier.

[BenHenning]

@vinitamurthi do you have some context on why we're setting real here instead of non-negative int?

[BenHenning]

Thanks Sean.

We should test these cross scenarios more thoroughly. Filed #449 to track.

[BenHenning]

Fix is forthcoming.

[seanlip]

Agreed this is serious, we should be able to respond gracefully to all possible inputs. Should we put #449 on a priority list?

[BenHenning]

Yep, it's essential. I think we're incorrectly parsing other interaction inputs as well, we just probably don't run into those. JSON is not the best way to be doing this, so we have a lot of work to do around improving our data typing throughout the retrieval pipeline to make these sorts of issues much harder to hit, plus reach 100% behavioral test coverage so that we detect when it does happen.

[vinitamurthi]

A numerator can be a negative int as well right? That's why it was set as real, otherwise we could have a negative int as well

[vinitamurthi]

My comment got deleted - The JSON solution is only temporary for these 6 explorations right? If so, the only rule specs I came across that were different from the original input were HasNumeratorEqualTo and HasDenominatorEqualTo.

[BenHenning]

The fix from #450 seems to sometimes not work. I need to investigate further.