Enable tutorial.error.checker to provide checks on error

[garrettgman]

If a student submits code that throws an error, learnr exits the checking routine and displays the error message. This short circuit prevents the checking code from telling the student anything useful about what might have gone wrong. In short, the student is left to decipher the error message on his or her own.

This is necessary when the checking code relies on the result of the student code (if it throws an error, there is no result). But it is a missed opportunity when the checking code parses the unevaluated student code, as gradethis::grade_code() does. Here is an example, grade_code() would return "I expected mtcars where you wrote mt" but learnr displays the error message. (code appended at bottom)

To fix this, I suggest that we:

1. Add a tutorials.error.checker option that can be set to a grading function to run in the event that the student code throws an error. This option can be set in the same manner as the tutorials.exercise.checker option. It should be set to a function that can grade the student code without evaluating it, similar to gradethis::grade_code().

2. Display the result of the error checking function beneath the error message. Here is a mock up.

Code to recreate existing example above

---

---
title: "Example"
output: learnr::tutorial
runtime: shiny_prerendered
---

```{r setup, include=FALSE}
library(learnr)
library(gradethis)

tutorial_options(exercise.checker = gradethis::grade_learnr)
knitr::opts_chunk$set(out.width = "100%")
```

## Welcome

### `mtcars`

To see the `mtcars` data frame, type `mtcars` in the code block below and click "Submit Answer".

```{r ex, exercise = TRUE}

```

```{r ex-solution}
mtcars
```

```{r ex-check}
grade_code("Good job!")
```

[garrettgman]

At the moment, this is the chief reason I'm delaying wiring up an autograder to the primers.

[schloerke]

Plan of action:

• Have learnr determine if it is a syntax error. Allow for global option to set message of syntax error.
• If an evaluation error, call -check chunk with the result as the error object. The grading code should handle the error object.

[dtkaplan]

I endorse Garrett's point about enabling the grading function to handle parsing errors and evaluation errors. Let me describe a strategy that I think would add a lot of flexibility and power to people writing error-checking packages. You can leave the current exercise=TRUE system and all its fellow -hint, -solution, -check etc. chunks as they are.

Create another type of user-input widget. Let's call in it a submission=TRUE object. It could look identical to the exercise widget but doesn't have to. When the user presses "Run code" or "hint" or "submit", the learnr system would set up a data structure and hand that off to an author-specified handle_submission() function. The data structure should include:

1. The content that the student put in the text field.
2. Which button the student pressed: "hint" or "submit" or "run code."
3. A list with the content of all chunks in the document, along with the chunk label and a list of chunk parameters.
4. A uiOutput object associated with the chunk and located just below it. (And, I'd suggest another one just above it to replace the current 'hint' display. But leave that aside for the moment.)

NOTE ADDED AS I EXPLORE THE ISSUE MORE ... @garrettgman I wonder if I can implement this just by providing a new function for getOption("tutorial.exercise.evaluator"). Something along the lines of learnr::inline_evaluator

Leave it to the exercise checker to evaluate any setup code from (3) and the student submission code (1).

Learnr should provide functions that allows the checker system to assign specific shiny output widgets to the uiOutput associated with the submission chunk, and write to those widgets in the usual shiny way.

Learnr should not evaluate the student submission directly. Leave that to the checker code, with learnr providing a safe sandbox in which that checker code can operate.

My experience in writing checkr, which was possibly the first learnr checking package, was that I don't need learnr to be providing environments and values, etc. I can do all that more effectively in the checking code.

Maybe a short way to describe what I'd like ...

• a way to display a module of shiny inputs and outputs
• a function that will return the contents of all chunks in the document, along with labels, etc.
• a safe place to evaluate the server components of the shiny module
• a way to trigger a learnr event so that the results of the checking can become part of the stream of logging events from the learnr document.

[garrettgman]

The fix in #403 solves a different issue:

• IIUC Checking for exercise errors #403 provides a way to check exercises whose correct result is an error. It implements a system that safely avoids evaluating code submissions during the check. As a result, you can check code without triggering the error it attempts to generate. As a trade-off, you cannot check the result of the code—but here you assume the result will be an error.

• This issue requires a way to check code that is supposed to return a result, but instead returns an error. For any exercise, a student may accidentally submit bad code that returns an error. If they do, this short-circuits the checking process and the student does not receive feedback about their code.

  Here, teachers cannot decide ahead of time to use an alternate checking system that does not evaluate student code, because teachers do want to evaluate student code and run checks against the result. What teachers need is a way to catch errors if they happen and drop into a "plan b" for checking the code that returns the error. The checking process should not short-circuit so learnr can display an error message. It should analyze the bad code and generate a feedback message to pass on to the student, along with the correct/incorrect status.

In other words, we need to keep the method of checking that evaluates student code to check the result, but we need to supplement it with a routine that spots and handles errors without exiting the checking algorithm.

@cpsievert thanks for your hard work, and let me know if I misunderstood #403, because it is very possible.

[cpsievert]

IIUC #403 provides a way to check exercises whose correct result is an error....This issue requires a way to check code that is supposed to return a result, but instead returns an error.

#403 addresses both of these needs. Here a link to an example of the former case.

And here's an example of the latter case:

```{r setup, include=FALSE}
library(learnr)
library(gradethis)

tutorial_options(
  exercise.checker = gradethis::grade_learnr,
  exercise.error.checker = gradethis::grade_learnr_error
)
knitr::opts_chunk$set(echo = FALSE)
```

* Enter `mtcars`

```{r grade_error, exercise = TRUE}

```

```{r grade_error-solution}
mtcars
```

```{r grade_error-code-check}
grade_code()
```

[cpsievert]

BTW, I don't know what you mean by checking algorithm, but if it happens to want to be something other than grade_code(), you can supply something other than grade_learnr_error (which ends up calling grade_code() if there is any solution code) to exercise.error.checker, which has access to the same information as grade_learnr() (i.e., user code, solution code, environments, etc).

BTW, we also now have access to the error condition object in .result within the -check chunk, so you can do something like (I'm not sure if this is actually useful or not):

```{r grade_error, exercise = TRUE}

```

```{r grade_error-check}
grade_result(
  fail_if(~inherits(.result, "simpleError"), message = "Some custom message.") 
)
```

If you happen to need a different checking algorithm than grade_code(), let's discuss that in another issue, but as far as having the ability to provide custom feedback when a exercise submission yields an error, #403 (and rstudio/gradethis#123) provide the ability to do (regardless of what the solution is)

[garrettgman]

@cpsievert Thanks for the quick reply. I need something slightly different for the second case.

To make it concrete, I need to use both -code and -code-chunk-like behaviors:

```{r setup, include=FALSE}
library(learnr)
library(gradethis)

tutorial_options(
  exercise.checker = gradethis::grade_learnr,
  exercise.error.checker = gradethis::grade_learnr_error
)
knitr::opts_chunk$set(echo = FALSE)
```

* Enter `mtcars`

```{r grade_error, exercise = TRUE}

```

```{r grade_error-solution}
mtcars
```

```{r grade_error-check}
grade_result(
  fail_if(~identical(.result, cars), "Good guess. Here's why you made that mistake.")
)
```

```{r grade_error-code-check}
grade_code()
```

If the student enters something that produces an error, like mt, I need the behavior you displayed above. But if the student submits valid R code, like cars, I need the -code behavior like this.

Under the current setup, I get the -code-chunk behavior even if they submit valid code.

This is what I mean by a "plan b": we first try to check against the result with -check, only if that is impossible do we retry with -code-chunk.

I'd like to track this as an open issue. But I can open a new issue for it if you prefer.

[garrettgman]

I think of the `-check` chunk (and `grade_result()` ) as the grading equivalent of unit tests. I'm trying to anticipate ahead of time all of the ways the student can fail, and to provide targeted guidance for each failure mode. Here, `cars` is not the right answer, but it is a failure mode that I want to screen for, because the two data set names are easy to confuse in your head (BTW this is a made up example). Most "unit tests" will need to test the result of the student code. But the final "unit test" would always be "was the result an error? If so return the output of `grade_code()`." The tricky thing is to make sure that _all_ of the tests get applied to the student work in every case. TL;DR - I want them to get mtcars because that is the solution, but I want to include a check for the failure mode where they use cars.

…

On Fri, Aug 21, 2020 at 7:49 PM Carson Sievert ***@***.***> wrote: I don't quite follow...why do you have mtcars in the -solution chunk, but cars in the -check chunk? — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#356 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAUYFXQLC76OBYXQECSX2TSB4B2LANCNFSM4MQHZ7DA> .

[cpsievert]

But the final "unit test" would always be "was the result an error? If so return the output of grade_code()."

Sounds like this implementation of exercise.error.checker does want you want (let me know if it doesn't):

function(solution_code = NULL, check_code, ...) {
   if (is.null(solution_code)) {
     return(NULL)
   }
  grade_learnr(solution_code = solution_code, check_code = "grade_code()", ...)
}

This'll make sure grade_code() is run when an evaluation error occurs and a solution is provided. This is different from the current implementation of grade_learnr_error (maybe we should change it?), which currently runs the -check chunk code on the evaluation error.

function(solution_code = NULL, check_code, ...) {
   if (is.null(solution_code)) {
     return(NULL)
   }
  grade_learnr(solution_code = solution_code, check_code = check_code, ...)
}