search

suwoons / pe

0 stars 0 forks source link

Inaccurate error message #6

Open suwoons opened 4 years ago

suwoons commented 4 years ago

The error was that new was not included, but the error message said that it was the prefix -t that was missing instead.

image.png

nus-pe-bot commented 4 years ago

Team's Response

We understand that this behaviour might be confusing, but this is actually the intended behaviour of our corrections feature. Let me explain how our correction feature works:

  1. Firstly, we need to look at the concept of edit distance. It refers to the distance between two words.
  2. Now, this concept of edit distance forms the cornerstone of our corrections feature. We calculate the distance between the user input and the valid commands our system accepts, and check if they are within some certain threshold distance (and of course we correct the user input to the valid command with the closest edit distance).
  3. We have configured Notably to have a distance threshold of 2.

NB: There are some additional rules that we've included into our corrections feature that allows us to correct "dl" into "delete" and "s" into "search", for example, that we'll not discuss about here since it'll make this response super lengthy.

This heuristic of course lead to some small subset of strange behaviours, such as the one reported here. In this case, the input "cb" got corrected to the relative path ".", since their distance is 2. That's why the current note is deleted, as shown in the video.

We are fully aware of this issue while designing our corrections feature and we know that this affects the user experience of our application (that's why we're accepting this issue). We believe, however, that the benefits our corrections feature provide in building a fluid typing experience much outweighs the drawbacks it poses (such as this strange behaviour reported here, for example).

As much as we like the corrections feature that we've built (the edit distance concept is pretty neat right!), it is at the end of the day just a collection of rules on how to correct some user input to a valid command. That's exactly the reason why, we believe, state of the art corrections feature (for example, the one built into your phones) are usually built with machine learning. Hand-defined set of rules will hardly ever be able to cover all sorts of edge cases!

All of this is to say that we are aware of such drawback in our feature, yet we don't think that we should be penalized heavily (if at all) for it!

Items for the Tester to Verify

:question: Issue severity

Team chose [severity.Low] Originally [severity.Medium]

Reason for disagreement: I think this response was intended for another issue, because the cb input or video they mention is irrelevant to this bug report.

Putting that aside, I don't think an explanation for how the correction engine works adequately justifies changing the severity of the bug. While I understand the utility of the correction engine in cases when there is a typo in a command, using it to point out (assumed) errors in a command is still rather problematic, as can be seen in this example. It does not negate the inconvenience posed to the user of having to figure out what the true bug is (missing a new command), and in fact further confuses them due to the inaccuracy. As described, a medium severity bug is "A flaw that causes occasional inconvenience to some users but they can continue to use the product" and I believe that is appropriate for this bug.