feat: improve timeline reordering

components/timeline/TimelineHome.vue

@@ -8,6 +8,6 @@ onBeforeUnmount(() => stream?.then(s => s.disconnect()))
 <template>
   <div>
     <PublishWidget draft-key="home" border="b base" />
-    <TimelinePaginator v-bind="{ paginator, stream }" :preprocess="timelineWithReorderedReplies" context="home" />
+    <TimelinePaginator v-bind="{ paginator, stream }" :preprocess="reorderedTimeline" context="home" />
   </div>
 </template>

composables/masto.ts

@@ -158,20 +158,3 @@ async function fetchRelationships() {
   for (let i = 0; i < requested.length; i++)
     requested[i][1].value = relationships[i]
 }
-
-const maxDistance = 10
-export function timelineWithReorderedReplies(items: Status[]) {
-  const newItems = [...items]
-  // TODO: Basic reordering, we should get something more efficient and robust
-  for (let i = items.length - 1; i > 0; i--) {
-    for (let k = 1; k <= maxDistance && i - k >= 0; k++) {
-      const inReplyToId = newItems[i - k].inReplyToId ?? newItems[i - k].reblog?.inReplyToId
-      if (inReplyToId && (inReplyToId === newItems[i].reblog?.id || inReplyToId === newItems[i].id)) {
-        const item = newItems.splice(i, 1)[0]
-        newItems.splice(i - k, 0, item)
-        k = 1
-      }
-    }
-  }
-  return newItems
-}

composables/timeline.ts

@@ -0,0 +1,50 @@
+import type { Status } from 'masto'
+
+const maxDistance = 10
+const maxSteps = 1000
+
+// Checks if (b) is a reply to (a)
+function areStatusesConsecutive(a: Status, b: Status) {
+  const inReplyToId = b.inReplyToId ?? b.reblog?.inReplyToId
+  return !!inReplyToId && (inReplyToId === a.reblog?.id || inReplyToId === a.id)
+}
+
+export function reorderedTimeline(items: Status[]) {
+  let steps = 0
+  const newItems = [...items]
+  for (let i = items.length - 1; i > 0; i--) {
+    for (let k = 1; k <= maxDistance && i - k >= 0; k++) {
+      // Prevent infinite loops
+      steps++
+      if (steps > maxSteps)
+        return newItems
+
+      // Check if the [i-k] item is a reply to the [i] item
+      // This means that they are in the wrong order
+
+      if (areStatusesConsecutive(newItems[i], newItems[i - k])) {
+        const item = newItems.splice(i, 1)[0]
+        newItems.splice(i - k, 0, item) // insert older item before the newer one
+        k = 0
+      }
+      else if (k > 1) {
+        // Check if the [i] item is a reply to the [i-k] item
+        // This means that they are in the correct order but there are posts between them
+        if (areStatusesConsecutive(newItems[i - k], newItems[i])) {
+          // If the next statuses are already ordered, move them all
+          let j = i
+          for (; j < items.length - 1; j++) {
+            if (!areStatusesConsecutive(newItems[j], newItems[j + 1]))
+              break
+          }
+          const orderedCount = j - i + 1
+          const itemsToMove = newItems.splice(i, orderedCount)
+          // insert older item after the newer one
+          newItems.splice(i - k + 1, 0, ...itemsToMove)
+          k = 0
+        }
+      }
+    }
+  }
+  return newItems
+}

pages/[[server]]/@[account]/index/index.vue

@@ -23,6 +23,6 @@ if (account) {
 <template>
   <div>
     <AccountTabs />
-    <TimelinePaginator :paginator="paginator" :preprocess="timelineWithReorderedReplies" context="account" />
+    <TimelinePaginator :paginator="paginator" :preprocess="reorderedTimeline" context="account" />
   </div>
 </template>

pages/[[server]]/@[account]/index/media.vue

@@ -21,6 +21,6 @@ if (account) {
 <template>
   <div>
     <AccountTabs />
-    <TimelinePaginator :paginator="paginator" :preprocess="timelineWithReorderedReplies" context="account" />
+    <TimelinePaginator :paginator="paginator" :preprocess="reorderedTimeline" context="account" />
   </div>
 </template>

pages/[[server]]/@[account]/index/with_replies.vue

@@ -21,6 +21,6 @@ if (account) {
 <template>
   <div>
     <AccountTabs />
-    <TimelinePaginator :paginator="paginator" :preprocess="timelineWithReorderedReplies" context="account" />
+    <TimelinePaginator :paginator="paginator" :preprocess="reorderedTimeline" context="account" />
   </div>
 </template>

tests/reorder-timeline.test.ts

@@ -0,0 +1,65 @@
+/**
+ * @vitest-environment jsdom
+ */
+import type { Status } from 'masto'
+import { describe, expect, it } from 'vitest'
+import { reorderedTimeline } from '~/composables/timeline'
+
+function status(id: string): Status {
+  return { id } as Status
+}
+function reply(id: string, s: Status) {
+  return { id, inReplyToId: s.id } as Status
+}
+function reblog(id: string, s: Status) {
+  return { id, reblog: s } as Status
+}
+
+const p_a1 = status('p_a1')
+const p_b1 = status('p_b1')
+
+const p_a2 = reply('p_a2', p_a1)
+const p_b2 = reply('p_b2', p_b1)
+
+const p_a3 = reply('p_a3', p_a2)
+const p_b3 = reply('p_b3', p_b2)
+
+const r_a1 = reblog('r_a1', p_a1)
+const r_b1 = reblog('r_b1', p_b1)
+
+const r_a2 = reblog('r_a2', p_a2)
+const r_b2 = reblog('r_b2', p_b2)
+
+describe('timeline reordering', () => {
+  it('reorder basic', () => {
+    expect(reorderedTimeline([p_a1, p_a2, p_a3]))
+      .toMatchInlineSnapshot([p_a1, p_a2, p_a3])
+
+    expect(reorderedTimeline([p_a3, p_a2, p_a1]))
+      .toMatchInlineSnapshot([p_a1, p_a2, p_a3])
+
+    expect(reorderedTimeline([p_a2, p_a3, p_a1]))
+      .toMatchInlineSnapshot([p_a1, p_a2, p_a3])
+
+    expect(reorderedTimeline([p_a2, p_b3, p_a3, p_b1, p_a1, p_b2]))
+      .toMatchInlineSnapshot([p_a1, p_a2, p_a3, p_b1, p_b2, p_b3])
+
+    expect(reorderedTimeline([r_a2, p_a1]))
+      .toMatchInlineSnapshot([p_a1, r_a2])
+
+    expect(reorderedTimeline([r_a2, p_b3, p_a3, p_b1, p_a1, r_b2]))
+      .toMatchInlineSnapshot([p_a1, r_a2, p_a3, p_b1, r_b2, p_b3])
+
+    expect(reorderedTimeline([r_a2, p_b3, p_a3, p_b1, p_a1, r_b2]))
+      .toMatchInlineSnapshot([p_a1, r_a2, p_a3, p_b1, r_b2, p_b3])
+
+    expect(reorderedTimeline([p_a1, p_b1, p_a2, p_b2, p_a3, p_b3]))
+      .toMatchInlineSnapshot([p_a1, p_a2, p_a3, p_b1, p_b2, p_b3])
+
+    expect(reorderedTimeline([r_a2, r_a1]))
+      .toMatchInlineSnapshot([r_a1, r_a2])
+
+    expect(reorderedTimeline([p_a3, r_a1, r_a2, r_b2, p_b3, r_b1]))
+      .toMatchInlineSnapshot([r_a1, r_a2, p_a3, r_b1, r_b2, p_b3])
+  })
+})