Repository: microsoft/pybryt

Issue:
Named annotation message filtering does not work correctly when no failure message is supplied
If no failure message is supplied but a success message is in a group of named annotations and one of the annotations is not satisfied, the success message is shown even though the reference registers as not being satisfied. In this case, no message should be shown.

Example:

```python
max_ref = []
def maximum(l, track=False):
    m = max(l)
    if track:
        max_ref.append(pybryt.Value(m, name="list-maximum", success_message="Found the max!")
    return m

test_lists = [[1, 2, 3], [-1, 0, 1], [10, -4, 2, 0], [1]]
for test_list in test_lists:
    maximum(test_list, track=True)

max_ref = pybryt.ReferenceImplementation("maximum", max_ref)

def maximum(l):
    if len(l) % 2 == 0:
        m = min(l)
    else:
        m = max(l)
    return m

with pybryt.check(max_ref):
    for test_list in test_lists:
        maximum(test_list)
```

Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index aaaba8f..82de47d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,6 +7,8 @@ All notable changes to this project will be documented in this file, and this pr
 
 * Added the `RequireImport` and `ForbidImport` annotations per [#123](https://github.com/microsoft/pybryt/issues/123)
 * Added `TimeComplexityChecker` for analyzing complexity without annotations per [#137](https://github.com/microsoft/pybryt/issues/137)
+* Fixed annotation message filtering bug per [#145](https://github.com/microsoft/pybryt/issues/145)
+* Added `dataclasses` backport to required modules for Python &lt; 3.7
 
 ## 0.3.1 - 2021-12-01
 
diff --git a/pybryt/annotations/annotation.py b/pybryt/annotations/annotation.py
index c3e1633..f386995 100644
--- a/pybryt/annotations/annotation.py
+++ b/pybryt/annotations/annotation.py
@@ -3,7 +3,7 @@
 __all__ = ["Annotation", "AnnotationResult"]
 
 from abc import ABC, abstractmethod
-from collections import OrderedDict
+from dataclasses import dataclass
 from typing import Any, Dict, List, Optional, Tuple
 
 from ..execution import is_complexity_tracing_enabled, MemoryFootprint
@@ -270,6 +270,26 @@ class Annotation(ABC):
         }
 
 
+@dataclass
+class AnnotationResultMessage:
+    """
+    A data class representing a success or failure message generated by an annotation.
+
+    Args:
+        message (``Optional[str]``): the message text
+        name (``Optional[str]``): the annotation name
+        satisfied (``bool``): whether the annotation providing the message was satisfied
+    """
+    message: Optional[str]
+    """the message text"""
+
+    name: Optional[str]
+    """the annotation name"""
+
+    satisfied: bool
+    """whether the annotation was satisfied"""
+
+
 class AnnotationResult:
     """
     Class that manages and defines an API for interacting with the results of an annotation.
@@ -369,21 +389,20 @@ class AnnotationResult:
         return self._value
 
     @property
-    def messages(self) -> List[Tuple[str, Optional[str], bool]]: # (message, name, satisfied)
+    def messages(self) -> List[AnnotationResultMessage]:
         """
-        ``list[tuple[str, Optional[str], bool]]``: The messages returned by this annotation and its 
-        children based on whether or not the annotations were satisfied. A list of tuples where the 
-        first element is the message, the second is the name of the annotation (or ``None`` if no 
-        name is  present), and the third is whether the annotation was satisfied.
+        ``list[AnnotationResultMessage]``: the messages returned by this annotation and its 
+        children based on whether or not the annotations were satisfied
         """
         messages = []
         for c in self.children:
             messages.extend(c.messages)
-        
-        if self.satisfied and self.annotation.success_message:
-            messages.append((self.annotation.success_message, self.annotation.name, True))
-        elif not self.satisfied and self.annotation.failure_message:
-            messages.append((self.annotation.failure_message, self.annotation.name, False))
+
+        messages.append(AnnotationResultMessage(
+            self.annotation.success_message if self.satisfied else self.annotation.failure_message,
+            self.annotation.name,
+            self.satisfied,
+        ))
 
         return messages
 
diff --git a/pybryt/reference.py b/pybryt/reference.py
index 6592a49..a635c10 100644
--- a/pybryt/reference.py
+++ b/pybryt/reference.py
@@ -234,18 +234,18 @@ class ReferenceResult(Serializable):
         implementation; if ``self.group`` is not ``None``, only messages from annotations in that
         group are returned
         """
-        messages = []
-        message_names = {}
+        finalized_messages, message_indices_by_name = [], {}
         for r in self.results:
-            m = r.messages
-            for msg in m:
-                if msg[1] in message_names and not msg[2]:
-                    idx = message_names[msg[1]]
-                    messages[idx] = msg[0]
-                elif msg[1] not in message_names:
-                    message_names[msg[1]] = len(messages)
-                    messages.append(msg[0])
-        return messages
+            for msg in r.messages:
+                if msg.name in message_indices_by_name and not msg.satisfied:
+                    idx = message_indices_by_name[msg.name]
+                    finalized_messages[idx] = msg.message
+
+                elif msg.name not in message_indices_by_name:
+                    message_indices_by_name[msg.name] = len(finalized_messages)
+                    finalized_messages.append(msg.message)
+
+        return [m for m in finalized_messages if m is not None]
 
     def to_dict(self) -> Dict[str, Any]:
         """
diff --git a/setup.py b/setup.py
index 2b886f7..9e0b6a6 100644
--- a/setup.py
+++ b/setup.py
@@ -1,4 +1,5 @@
 import setuptools
+import sys
 
 with open("README.md", "r") as fh:
 	long_description = fh.read()
@@ -6,6 +7,10 @@ with open("README.md", "r") as fh:
 with open("requirements.txt") as f:
 	install_requires = f.readlines()
 
+# add dataclasses backport for python 3.6
+if sys.version_info.minor < 7:
+	install_requires.append("dataclasses")
+
 # get version
 env = {}
 with open("pybryt/version.py") as f:


Test patch:
diff --git a/tests/annotations/test_annotation.py b/tests/annotations/test_annotation.py
index aa13029..96f0b14 100644
--- a/tests/annotations/test_annotation.py
+++ b/tests/annotations/test_annotation.py
@@ -82,29 +82,27 @@ def test_messages():
     v = v1.before(v2)
     res = v.check(footprint)
 
-    assert len(res.messages) == 1, "Too many messages"
-    assert res.messages[0] == ("m1", 'Annotation 1', True), "Wrong message"
+    assert len(res.messages) == 3
+    assert_object_attrs(res.messages[0], {"message": "m1", "satisfied": True})
 
     v.failure_message = "m3"
     res = v.check(footprint)
 
-    assert len(res.messages) == 1, "Too many messages"
-    assert res.messages[0] == ("m1", 'Annotation 1', True), "Wrong message"
+    assert len(res.messages) == 3
 
     res = v.check(MemoryFootprint.from_values())
 
-    assert len(res.messages) == 2, "Wrong number of messages"
-    assert res.messages[0] == ("m2", 'Annotation 1', False), "Wrong message"
-    assert res.messages[1] == ("m3", 'Annotation 3', False), "Wrong message"
+    assert len(res.messages) == 3
+    assert_object_attrs(res.messages[0], {"message": "m2", "satisfied": False})
+    assert_object_attrs(res.messages[1], {"message": None, "satisfied": False})
+    assert_object_attrs(res.messages[2], {"message": "m3", "satisfied": False})
 
     v2.name = "v2"
     v2.failure_message = "m4"
     res = v.check(MemoryFootprint.from_values())
 
-    assert len(res.messages) == 3, "Wrong number of messages"
-    assert res.messages[0] == ("m2", 'Annotation 1', False), "Wrong message"
-    assert res.messages[1] == ("m4", "v2", False), "Wrong message"
-    assert res.messages[2] == ("m3", 'Annotation 3', False), "Wrong message"
+    assert len(res.messages) == 3
+    assert_object_attrs(res.messages[1], {"message": "m4", "name": "v2", "satisfied": False})
 
 
 def test_bitwise_ops():
diff --git a/tests/test_reference.py b/tests/test_reference.py
index b0123e7..e77644b 100644
--- a/tests/test_reference.py
+++ b/tests/test_reference.py
@@ -228,6 +228,25 @@ def test_run_and_results():
     with pytest.raises(ValueError, match="Group 'foo' not found"):
         ref.run(footprint, group="foo")
 
+    # check message filtering (#145)
+    ref = ReferenceImplementation("foo", [
+        Value(1, name="1", success_message="sm1"),
+        Value(2, name="1", success_message="sm1"),
+        Value(3, name="2", failure_message="fm2"),
+        Value(4, name="2", failure_message="fm2"),
+        Value(5, name="3", success_message="sm3", failure_message="fm3"),
+        Value(6, name="3", success_message="sm3", failure_message="fm3"),
+    ])
+
+    res = ref.run(MemoryFootprint.from_values(1, 1, 2, 2, 3, 3, 4, 4, 5, 5, 6, 6))
+    assert res.messages == ["sm1", "sm3"]
+
+    res = ref.run(MemoryFootprint.from_values(1, 1, 3, 3, 4, 4))
+    assert res.messages == ["fm3"]
+
+    res = ref.run(MemoryFootprint.from_values(1, 1, 2, 2, 5, 5))
+    assert res.messages == ["sm1", "fm2", "fm3"]
+
 
 def test_generate_report():
     """