From caafda955314e9504b366b23796e3f9ff81a78d2 Mon Sep 17 00:00:00 2001 From: Coracle-Bot Date: Wed, 9 Sep 2026 02:30:02 +0000 Subject: [PATCH] Break same-second event ties by id so every client agrees on order --- e2e/USER_STORIES.md | 10 ++++++ e2e/specs/rooms.spec.ts | 41 ++++++++++++++++++++++ src/app/feeds.ts | 76 +++++++++++++++++------------------------ 3 files changed, 83 insertions(+), 44 deletions(-) diff --git a/e2e/USER_STORIES.md b/e2e/USER_STORIES.md index 1d28cfc0..cdc72c97 100644 --- a/e2e/USER_STORIES.md +++ b/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 diff --git a/e2e/specs/rooms.spec.ts b/e2e/specs/rooms.spec.ts index ee4ee578..dc576d13 100644 --- a/e2e/specs/rooms.spec.ts +++ b/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") diff --git a/src/app/feeds.ts b/src/app/feeds.ts index ff05dde4..8da944ec 100644 --- a/src/app/feeds.ts +++ b/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 = (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 => { - 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 - }) + events.update($events => mergeSorted($events, added, compareEventsAsc)) } } @@ -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( - getStart, - uniqBy( - e => e.id, - relays.flatMap(url => Array.from(getEventsForUrl(url, filters))), - ), - ), + uniqBy( + e => e.id, + relays.flatMap(url => Array.from(getEventsForUrl(url, filters))), + ).sort(compareByStart), ) 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) { - if (getStart(kept[i]) <= getStart(valid[j])) { - merged.push(kept[i++]) - } else { - merged.push(valid[j++]) - } - } - - while (i < kept.length) merged.push(kept[i++]) - while (j < valid.length) merged.push(valid[j++]) - - return merged + return mergeSorted( + $events.filter(e => !superseded.has(getAddress(e))), + valid, + compareByStart, + ) }) }