Repository: RasaHQ/rasa
Pull request: Remove incorrect predictions from NLU dump at end of interactive learning
Fixes https://github.com/RasaHQ/rasa/issues/3956

When an nlu correction was made, the event is rewound but we never check for this when dumping new data.  This would dump incorrect data and additionally cause errors (e.g when there is no confident prediction, rasa tries to dump `"intent": None`)

**Proposed changes**:
- Check for the `rewind` event when dumping and remove the latest user message if it is found

**Status (please check what you already did)**:
- [x] made PR ready for code review
- [ ] added some tests for the functionality
- [ ] updated the documentation
- [x] updated the changelog
- [x] reformat files using `black` (please check [Readme](https://github.com/RasaHQ/rasa_nlu#code-style) for instructions)


Problem statement:
Interactive learning fails when the intent name and entity type are the same
When dumping NLU data at the end, if there is an entity type that is the same as an intent name, the following error is thrown up:

```bash
2019-07-08 12:51:09 ERROR    rasa.core.training.interactive  - An exception occurred while recording messages.
Traceback (most recent call last):
  File "/Users/tom/Documents/RASA/rasa/rasa/core/training/interactive.py", line 1363, in record_messages
    endpoint, sender_id, sender_ids, plot_file
  File "/Users/tom/Documents/RASA/rasa/rasa/core/training/interactive.py", line 881, in _predict_till_next_listen
    action_name, policy, confidence, predictions, endpoint, sender_id
  File "/Users/tom/Documents/RASA/rasa/rasa/core/training/interactive.py", line 1020, in _validate_action
    is_correct = await _ask_questions(question, sender_id, endpoint)
  File "/Users/tom/Documents/RASA/rasa/rasa/core/training/interactive.py", line 312, in _ask_questions
    should_retry = await _ask_if_quit(sender_id, endpoint)
  File "/Users/tom/Documents/RASA/rasa/rasa/core/training/interactive.py", line 589, in _ask_if_quit
    await _write_data_to_file(sender_id, endpoint)
  File "/Users/tom/Documents/RASA/rasa/rasa/core/training/interactive.py", line 565, in _write_data_to_file
    await _write_nlu_to_file(nlu_path, events)
  File "/Users/tom/Documents/RASA/rasa/rasa/core/training/interactive.py", line 807, in _write_nlu_to_file
    f.write(nlu_data.as_markdown())
  File "/Users/tom/Documents/RASA/rasa/rasa/nlu/training_data/training_data.py", line 134, in as_markdown
    return MarkdownWriter().dumps(self)
  File "/Users/tom/Documents/RASA/rasa/rasa/nlu/training_data/formats/markdown.py", line 206, in dumps
    md += self._generate_training_examples_md(training_data)
  File "/Users/tom/Documents/RASA/rasa/rasa/nlu/training_data/formats/markdown.py", line 219, in _generate_training_examples_md
    key=lambda k: k["intent"],
  File "/Users/tom/Documents/RASA/rasa/rasa/nlu/training_data/formats/markdown.py", line 219, in <lambda>
    key=lambda k: k["intent"],
KeyError: 'intent'
```

An example of what would cause this error:
```
------
Chat History

 #    Bot                            You        
────────────────────────────────────────────────
 1    action_listen                             
────────────────────────────────────────────────
 2                   i want to eat [pizza](food) 
                               intent: food 1.00 
------
```

Hints from issue discussion:
@MetcalfeTom please always add issues to the maintenance board

Commit message: Merge branch 'master' into dumping-none-intents

Patch to review:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 4c1677159359..ddab83977b94 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -23,6 +23,8 @@ Removed
 
 Fixed
 -----
+- interactive learning bug where reverted user utterances were dumped to nlu data
+- ``MappingPolicy`` now works correctly when used as part of a PolicyEnsemble
 - validation no longer throws an error during interactive learning
 
 [1.1.6] - 2019-07-12
diff --git a/rasa/core/training/interactive.py b/rasa/core/training/interactive.py
index ea225da01081..442996700009 100644
--- a/rasa/core/training/interactive.py
+++ b/rasa/core/training/interactive.py
@@ -759,6 +759,9 @@ def _collect_messages(events: List[Dict[Text, Any]]) -> List[Message]:
             msg = Message.build(data["text"], data["intent"]["name"], data["entities"])
             msgs.append(msg)
 
+        elif event.get("event") == UserUtteranceReverted.type_name:
+            msgs.pop()  # user corrected the nlu, remove incorrect example
+
     return msgs
 
 
@@ -785,7 +788,17 @@ async def _write_stories_to_file(
     with open(export_story_path, append_write, encoding="utf-8") as f:
         for conversation in sub_conversations:
             parsed_events = rasa.core.events.deserialise_events(conversation)
-            s = Story.from_events(parsed_events)
+            applied_events = []
+
+            for i, event in enumerate(parsed_events):
+                if event.type_name == UserUtteranceReverted.type_name:
+                    # user corrected their intent, so we have to remove
+                    # the incorrect intent, rewind action and action_listen
+                    applied_events = applied_events[:-2]
+                else:
+                    applied_events.append(event)
+
+            s = Story.from_events(applied_events)
             f.write("\n" + s.as_story_string(flat=True))
 
 


Code review comments:
diff_hunk: @@ -23,6 +23,8 @@ Removed
 
 Fixed
 -----
+- interactive learning bug where reverted user utterances were dumped to nlu data
+- ``MappingPolicy`` now works correctly when used as part of a PolicyEnsemble
path: CHANGELOG.rst
text: @user1:
think this got added in by mistake, this is in 1.1.5 already

diff_hunk: @@ -785,7 +788,17 @@ def _collect_actions(events: List[Dict[Text, Any]]) -> List[Dict[Text, Any]]:
     with open(export_story_path, append_write, encoding="utf-8") as f:
         for conversation in sub_conversations:
             parsed_events = rasa.core.events.deserialise_events(conversation)
-            s = Story.from_events(parsed_events)
+            applied_events = []
+
+            for i, event in enumerate(parsed_events):
path: rasa/core/training/interactive.py
text: @user1:
hm is there a way we could make this code https://github.com/RasaHQ/rasa/blob/master/rasa/core/trackers.py#L335 through 362 reusable, instead of reinventing the wheel here? Maybe it should go in events instead of under trackers, e.g. a method that takes a list of events and returns applied ones, this can then be used in the tracker method. i'd prefer that method because it seems to handle UURs better -- the [:-2] has failed me before (I still have a PR floating somewhere because buttons in CLI don't show up after some UURs or restarts for this reason)

diff_hunk: @@ -23,6 +23,8 @@ Removed
 
 Fixed
 -----
+- interactive learning bug where reverted user utterances were dumped to nlu data
path: CHANGELOG.rst
text: @user1:
```suggestion
- interactive learning bug where reverted user utterances were dumped to training data
```
as they were dumped to stories too