Break same-second event ties by id so every client agrees on order

e2e/USER_STORIES.md

@@ -273,6 +273,16 @@ Acceptance:
 - bob, already viewing the same room in his own session, sees the message appear
   without reloading.
 
+### US-118 — Messages sent in the same second are in one order for everyone
+
+As alice, I want a room to read the same way for me as it does for bob, so that
+we can refer to what was said without first agreeing on what order it was in.
+
+Acceptance:
+
+- Five messages sharing one timestamp are shown in ascending event id order,
+  whatever order they reached the client in.
+
 ### US-019 — Join and leave a room
 
 As bob, I want to join a room's member list and leave it later, so that it shows

e2e/specs/rooms.spec.ts

@@ -175,6 +175,47 @@ test("US-018 send and receive a room message in real time", async ({seed, as}) =
   await expect(message(bob, "second line")).toContainText("first line")
 })
 
+// Two people typing in the same second used to leave every client with a different transcript,
+// since the timestamp alone gave the merge nothing to break the tie on and each client saw its own
+// message arrive first. Ordering falls back to the event id, which is the same everywhere.
+test("US-118 messages sent in the same second are in one order for everyone", async ({
+  seed,
+  as,
+}) => {
+  const tied: Seeded[] = []
+
+  const scenario = await seed(({relay, user, at}) => {
+    const space = relay("space")
+    const sentAt = at(2, HOUR)
+
+    space.room("general", {name: "General"})
+    space.join(user.alice, "general")
+    space.join(user.bob, "general")
+
+    tied.push(
+      space.message(user.bob, "general", "the tide turns at four", sentAt),
+      space.message(user.alice, "general", "the tug is already out", sentAt),
+      space.message(user.bob, "general", "we cast off before dark", sentAt),
+      space.message(user.alice, "general", "the pilot boat follows us", sentAt),
+      space.message(user.bob, "general", "and the harbor master knows", sentAt),
+    )
+  })
+
+  const {url} = scenario.space("space")
+  const page = await as(users.alice, roomPath(url, "general"))
+
+  await expect(message(page, "and the harbor master knows")).toBeVisible()
+  await expect(messages(page)).toHaveCount(5)
+
+  const rendered = await messages(page).evaluateAll(items =>
+    items.map(item => item.getAttribute("data-event")),
+  )
+
+  // The room reads newest first in the dom, so the ids run the other way from the order the feed
+  // holds them in
+  expect(rendered.reverse()).toEqual(tied.map(({id}) => id).sort())
+})
+
 test("US-019 join and leave a room", async ({seed, as}) => {
   const scenario = await seed(({relay, user, at}) => {
     const space = relay("space")

src/app/feeds.ts

@@ -1,11 +1,12 @@
 import {derived, get, readable, writable} from "svelte/store"
 import type {Readable, Writable} from "svelte/store"
-import {batch, call, int, ms, now, on, sleep, sortBy, uniqBy, MONTH, YEAR} from "@welshman/lib"
+import {batch, call, int, ms, now, on, sleep, uniqBy, MONTH, YEAR} from "@welshman/lib"
 import {
   COMMENT,
   DELETE,
   EVENT_TIME,
   addressTags,
+  compareEventsAsc,
   getAddress,
   getCommentFiltersForRoot,
   getIdOrAddress,
@@ -30,6 +31,21 @@ import {getEventsForUrl} from "@app/repository"
 
 const noEvents: TrustedEvent[] = []
 
+const mergeSorted = <T>(left: T[], right: T[], compare: (a: T, b: T) => number) => {
+  const merged: T[] = []
+  let i = 0
+  let j = 0
+
+  while (i < left.length && j < right.length) {
+    merged.push(compare(left[i], right[j]) <= 0 ? left[i++] : right[j++])
+  }
+
+  while (i < left.length) merged.push(left[i++])
+  while (j < right.length) merged.push(right[j++])
+
+  return merged
+}
+
 // Reactions, zaps and reports point at their subject with `e`/`a`. A NIP-22 comment instead
 // points at its thread *root* with `E`/`A`, so filing it by those tags puts a whole thread in
 // the root's bucket — which is the scope a reply count wants.
@@ -435,26 +451,9 @@ export const makeFeed = ({
     }
 
     if (added.length > 0) {
-      added.sort((a, b) => a.created_at - b.created_at)
+      added.sort(compareEventsAsc)
 
-      events.update($events => {
+      events.update($events => mergeSorted($events, added, compareEventsAsc))
-        const merged: TrustedEvent[] = []
-        let i = 0
-        let j = 0
-
-        while (i < $events.length && j < added.length) {
-          if ($events[i].created_at <= added[j].created_at) {
-            merged.push($events[i++])
-          } else {
-            merged.push(added[j++])
-          }
-        }
-
-        while (i < $events.length) merged.push($events[i++])
-        while (j < added.length) merged.push(added[j++])
-
-        return merged
-      })
     }
   }
 
@@ -553,14 +552,14 @@ export const makeCalendarFeed = ({
 
   const getEnd = (event: TrustedEvent) => parseInt(tagValue(tagSpec("end"), event.tags) || "")
 
+  const compareByStart = (a: TrustedEvent, b: TrustedEvent) =>
+    getStart(a) - getStart(b) || compareEventsAsc(a, b)
+
   const events = writable(
-    sortBy(
+    uniqBy(
-      getStart,
+      e => e.id,
-      uniqBy(
+      relays.flatMap(url => Array.from(getEventsForUrl(url, filters))),
-        e => e.id,
+    ).sort(compareByStart),
-        relays.flatMap(url => Array.from(getEventsForUrl(url, filters))),
-      ),
-    ),
   )
 
   const insertEvents = (newEvents: TrustedEvent[]) => {
@@ -573,28 +572,17 @@ export const makeCalendarFeed = ({
       onEvent?.(event)
     }
 
-    valid.sort((a, b) => getStart(a) - getStart(b))
+    valid.sort(compareByStart)
 
     events.update($events => {
       // Calendar events are addressable, so a new version supersedes the old one
       const superseded = new Set(valid.map(getAddress))
-      const kept = $events.filter(e => !superseded.has(getAddress(e)))
-      const merged: TrustedEvent[] = []
-      let i = 0
-      let j = 0
 
-      while (i < kept.length && j < valid.length) {
+      return mergeSorted(
-        if (getStart(kept[i]) <= getStart(valid[j])) {
+        $events.filter(e => !superseded.has(getAddress(e))),
-          merged.push(kept[i++])
+        valid,
-        } else {
+        compareByStart,
-          merged.push(valid[j++])
+      )
-        }
-      }
-
-      while (i < kept.length) merged.push(kept[i++])
-      while (j < valid.length) merged.push(valid[j++])
-
-      return merged
     })
   }