fix: TypeError traversing conversation tree with timestamp-less utterances

convokit/model/utteranceNode.py

@@ -16,7 +16,11 @@ def __init__(self, utt: Utterance):
         self.children = []
 
     def set_children(self, children: List["UtteranceNode"]):
-        self.children = sorted(children, key=lambda w: w.utt.timestamp)  # earliest to latest utt
+        # earliest to latest utt; utterances with no timestamp (None) are sorted
+        # last and never compared against each other or against numeric timestamps
+        self.children = sorted(
+            children, key=lambda w: (w.utt.timestamp is None, w.utt.timestamp or 0)
+        )
 
     def pre_order(self):
         """

convokit/tests/general/traverse_convo/test_traverse_convo.py

@@ -5,6 +5,7 @@
     construct_tree_corpus,
     construct_nonexistent_reply_to_corpus,
     construct_multiple_convo_id_corpus,
+    construct_no_timestamp_tree_corpus,
 )
 from convokit.tests.test_utils import reload_corpus_in_db_mode
 
@@ -66,6 +67,12 @@ def one_utt_convo(self):
         self.assertEqual([utt.id for utt in convo.traverse("postorder")], ["other"])
         self.assertEqual([utt.id for utt in convo.traverse("preorder")], ["other"])
 
+    def traverse_no_timestamps(self):
+        # sibling utterances without timestamps must not crash tree traversal
+        convo = self.no_timestamp_corpus.get_conversation("0")
+        bfs_traversal = [utt.id for utt in convo.traverse("bfs", as_utterance=True)]
+        self.assertEqual(bfs_traversal, ["0", "1", "2"])
+
     def reindex_corpus(self):
         original_convo_meta = {
             k: v for k, v in self.corpus.get_conversation("0").meta.to_dict().items()
@@ -111,6 +118,9 @@ def setUp(self) -> None:
         self.nonexistent_reply_to_corpus = reload_corpus_in_db_mode(
             construct_nonexistent_reply_to_corpus()
         )
+        self.no_timestamp_corpus = reload_corpus_in_db_mode(
+            construct_no_timestamp_tree_corpus()
+        )
 
     def test_broken_convos(self):
         self.broken_convos()
@@ -136,6 +146,9 @@ def test_conversation_id_to_leaf_paths(self):
     def test_one_utt_convo(self):
         self.one_utt_convo()
 
+    def test_traverse_no_timestamps(self):
+        self.traverse_no_timestamps()
+
     def test_reindex_corpus(self):
         self.reindex_corpus()
 
@@ -148,6 +161,7 @@ def setUp(self) -> None:
         self.corpus = construct_tree_corpus()
         self.multiple_convo_id_corpus = construct_multiple_convo_id_corpus()
         self.nonexistent_reply_to_corpus = construct_nonexistent_reply_to_corpus()
+        self.no_timestamp_corpus = construct_no_timestamp_tree_corpus()
 
     def test_broken_convos(self):
         self.broken_convos()
@@ -173,6 +187,9 @@ def test_conversation_id_to_leaf_paths(self):
     def test_one_utt_convo(self):
         self.one_utt_convo()
 
+    def test_traverse_no_timestamps(self):
+        self.traverse_no_timestamps()
+
     def test_reindex_corpus(self):
         self.reindex_corpus()
 

convokit/tests/general/traverse_convo/traverse_convo_helpers.py

@@ -82,6 +82,25 @@ def construct_nonexistent_reply_to_corpus():
     return corpus
 
 
+def construct_no_timestamp_tree_corpus():
+    # a valid reply-to tree where the root has multiple children and no
+    # utterance has a timestamp set (timestamp is Optional and defaults to None)
+    corpus = Corpus(
+        utterances=[
+            Utterance(
+                id="0", reply_to=None, conversation_id="0", speaker=Speaker(id="alice")
+            ),
+            Utterance(
+                id="1", reply_to="0", conversation_id="0", speaker=Speaker(id="alice")
+            ),
+            Utterance(
+                id="2", reply_to="0", conversation_id="0", speaker=Speaker(id="alice")
+            ),
+        ]
+    )
+    return corpus
+
+
 def construct_tree_corpus():
     corpus = Corpus(
         utterances=[