icub3d/decentcom

Fork 0

Bug: Messages display out of order / duplicated in channels #43

New issue

Closed

opened 2026-04-26 16:46:01 +00:00 by icub3d · 0 comments

icub3d commented

2026-04-26 16:46:01 +00:00

Owner

Migrated from GitHub issue icub3d/decentcom#56
Original Author: @icub3d
Original Date: 2026-04-16T18:43:48Z

Bug: Messages Display Out of Order / Duplicated in Channels

Overview

Channel messages can appear out of order or duplicated. There are three related bugs in serverStore.ts loadMoreMessages that interact with real-time WebSocket events.

Root Cause

Bug 1 — Stale `existing` overwrites WebSocket messages (most impactful)

In loadMoreMessages (client/src/stores/serverStore.ts, line 272), existing is captured from the store before the API fetch, then used inside the set() callback after the fetch resolves:

const existing = state.messages[channelId] ?? [];   // snapshot taken here
const page = await apiRequest(...);                 // WS messages may arrive here
set((prev) => ({
  messages: {
    [channelId]: [...existing, ...page.messages],   // stale snapshot — overwrites WS msgs!
  },
}));

Any MESSAGE_CREATE WebSocket events that arrive during the in-flight fetch are prepended to prev.messages[channelId] by handleGatewayEvent, but then immediately overwritten when loadMoreMessages resolves using the stale existing. The result is missing messages and apparent ordering gaps.

Bug 2 — No deduplication in `loadMoreMessages`

If loadMoreMessages is called twice concurrently (e.g., rapid scroll before the guard fires), both calls capture the same existing and fetch the same page. The set() calls produce [...existing, ...samePage, ...existing, ...samePage] — duplicate messages that after reverse() in MessageList appear as repeated blocks.

The MESSAGE_CREATE WS handler does deduplicate by ID, but loadMoreMessages does not.

Bug 3 — No re-sort after merge

After merging API results and live WS messages there is no normalisation step. If any message arrives with an ID that falls in the middle of the existing array (e.g., after a reconnect delivers a backlog), it is prepended to the front regardless of timestamp, producing an out-of-order sequence that reverse() cannot fix.

Requirements

loadMoreMessages must use prev.messages[channelId] (current state) inside set(), not the stale snapshot captured before the fetch
When merging API page results into the store, deduplicate by message ID
After any mutation (initial load, pagination, WS event), the channel's message array must be sorted by ID descending so MessageList.reverse() always yields a consistent oldest→newest order
Add a test for the concurrent-call deduplication case and the stale-write case

Design

Component Changes

client/src/stores/serverStore.ts — loadMoreMessages:

set((prev) => {
  const current = prev.messages[channelId] ?? [];
  const merged = [...current, ...page.messages];
  // deduplicate by id
  const seen = new Set<string>();
  const deduped = merged.filter((m) => {
    if (seen.has(m.id)) return false;
    seen.add(m.id);
    return true;
  });
  // keep newest-first (ULIDs sort lexicographically = chronologically)
  deduped.sort((a, b) => (a.id < b.id ? 1 : -1));
  return {
    messages: { ...prev.messages, [channelId]: deduped },
    hasMore: { ...prev.hasMore, [channelId]: page.has_more },
  };
});

The same sort-after-merge approach should be applied in the MESSAGE_CREATE WS handler for robustness (reconnect backlog).

Task List

Fix loadMoreMessages: use prev.messages[channelId] inside set() instead of stale existing
Add deduplication by ID when merging API page into current store state
Sort merged array by ID descending after every mutation in loadMoreMessages
Apply the same sort in the MESSAGE_CREATE WS handler for reconnect safety
Add Vitest unit tests covering: stale-write race, concurrent pagination dedup, WS backlog ordering

Test List

Unit test: WS message arriving during in-flight loadMoreMessages survives and appears in correct position
Unit test: two concurrent loadMoreMessages calls produce no duplicates
Unit test: WS backlog (messages arriving out of chronological order) is sorted correctly
Manual: open a channel, scroll up to load history, send messages from another client — verify no gaps or duplicates
Manual: disconnect/reconnect while a channel is open — verify backlog messages appear in correct order

Open Questions

Should the sort step live in a shared helper (e.g. sortMessages) to avoid duplication between loadMoreMessages and the WS handler?
The loadingMoreRef guard in MessageList.tsx prevents double-scroll-triggered calls at the component level; should a similar guard be added to the store function itself for defence in depth?

**Migrated from GitHub issue icub3d/decentcom#56** **Original Author:** @icub3d **Original Date:** 2026-04-16T18:43:48Z --- # Bug: Messages Display Out of Order / Duplicated in Channels ## Overview Channel messages can appear out of order or duplicated. There are three related bugs in `serverStore.ts` `loadMoreMessages` that interact with real-time WebSocket events. ## Root Cause ### Bug 1 — Stale `existing` overwrites WebSocket messages (most impactful) In `loadMoreMessages` (`client/src/stores/serverStore.ts`, line 272), `existing` is captured from the store **before** the API fetch, then used inside the `set()` callback **after** the fetch resolves: ```ts const existing = state.messages[channelId] ?? []; // snapshot taken here const page = await apiRequest(...); // WS messages may arrive here set((prev) => ({ messages: { [channelId]: [...existing, ...page.messages], // stale snapshot — overwrites WS msgs! }, })); ``` Any `MESSAGE_CREATE` WebSocket events that arrive during the in-flight fetch are prepended to `prev.messages[channelId]` by `handleGatewayEvent`, but then immediately overwritten when `loadMoreMessages` resolves using the stale `existing`. The result is **missing messages** and apparent ordering gaps. ### Bug 2 — No deduplication in `loadMoreMessages` If `loadMoreMessages` is called twice concurrently (e.g., rapid scroll before the guard fires), both calls capture the same `existing` and fetch the same page. The `set()` calls produce `[...existing, ...samePage, ...existing, ...samePage]` — **duplicate messages** that after `reverse()` in `MessageList` appear as repeated blocks. The `MESSAGE_CREATE` WS handler does deduplicate by ID, but `loadMoreMessages` does not. ### Bug 3 — No re-sort after merge After merging API results and live WS messages there is no normalisation step. If any message arrives with an ID that falls in the middle of the existing array (e.g., after a reconnect delivers a backlog), it is prepended to the front regardless of timestamp, producing an **out-of-order sequence** that `reverse()` cannot fix. ## Requirements - [ ] `loadMoreMessages` must use `prev.messages[channelId]` (current state) inside `set()`, not the stale snapshot captured before the fetch - [ ] When merging API page results into the store, deduplicate by message ID - [ ] After any mutation (initial load, pagination, WS event), the channel's message array must be sorted by ID descending so `MessageList.reverse()` always yields a consistent oldest→newest order - [ ] Add a test for the concurrent-call deduplication case and the stale-write case ## Design ### Component Changes **`client/src/stores/serverStore.ts`** — `loadMoreMessages`: ```ts set((prev) => { const current = prev.messages[channelId] ?? []; const merged = [...current, ...page.messages]; // deduplicate by id const seen = new Set<string>(); const deduped = merged.filter((m) => { if (seen.has(m.id)) return false; seen.add(m.id); return true; }); // keep newest-first (ULIDs sort lexicographically = chronologically) deduped.sort((a, b) => (a.id < b.id ? 1 : -1)); return { messages: { ...prev.messages, [channelId]: deduped }, hasMore: { ...prev.hasMore, [channelId]: page.has_more }, }; }); ``` The same sort-after-merge approach should be applied in the `MESSAGE_CREATE` WS handler for robustness (reconnect backlog). ## Task List - [ ] Fix `loadMoreMessages`: use `prev.messages[channelId]` inside `set()` instead of stale `existing` - [ ] Add deduplication by ID when merging API page into current store state - [ ] Sort merged array by ID descending after every mutation in `loadMoreMessages` - [ ] Apply the same sort in the `MESSAGE_CREATE` WS handler for reconnect safety - [ ] Add Vitest unit tests covering: stale-write race, concurrent pagination dedup, WS backlog ordering ## Test List - [ ] Unit test: WS message arriving during in-flight `loadMoreMessages` survives and appears in correct position - [ ] Unit test: two concurrent `loadMoreMessages` calls produce no duplicates - [ ] Unit test: WS backlog (messages arriving out of chronological order) is sorted correctly - [ ] Manual: open a channel, scroll up to load history, send messages from another client — verify no gaps or duplicates - [ ] Manual: disconnect/reconnect while a channel is open — verify backlog messages appear in correct order ## Open Questions - Should the sort step live in a shared helper (e.g. `sortMessages`) to avoid duplication between `loadMoreMessages` and the WS handler? - The `loadingMoreRef` guard in `MessageList.tsx` prevents double-scroll-triggered calls at the component level; should a similar guard be added to the store function itself for defence in depth?

icub3d

2026-04-26 16:46:01 +00:00

closed this issue
added the
area:api

type:bug

status:complete

area:ux
labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

icub3d/decentcom#43

No description provided.

Rows
Columns