From fbd43d026be1f408ad98c8ac7574ba1bcecfd831 Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Thu, 6 Aug 2026 20:52:58 +0200 Subject: [PATCH 1/7] fix: properly respect reset and keepPreviousItems --- src/pagination/paginators/BasePaginator.ts | 31 ++++++---- .../paginators/BasePaginator.test.ts | 61 ++++++++++++++++--- 2 files changed, 70 insertions(+), 22 deletions(-) diff --git a/src/pagination/paginators/BasePaginator.ts b/src/pagination/paginators/BasePaginator.ts index 558cd3ea2..6ca0e58c0 100644 --- a/src/pagination/paginators/BasePaginator.ts +++ b/src/pagination/paginators/BasePaginator.ts @@ -2395,29 +2395,34 @@ export abstract class BasePaginator { silent, updateState = true, }: PaginationQueryParams = {}): Promise | void> { + if (reset === 'yes' && !forcedQueryShape) { + this.state.partialNext({ + cursor: this.config.initialCursor, + offset: this.config.initialOffset ?? 0, + }); + } const queryShape = forcedQueryShape ?? this.getNextQueryShape({ direction }); if (!this.canExecuteQuery({ direction, reset })) return; const isFirstPage = this.isFirstPageQuery({ queryShape, reset }); - if (isFirstPage && !keepPreviousItems) { + + if (isFirstPage && (!keepPreviousItems || reset === 'yes')) { const state = this.getStateBeforeFirstQuery(); if (reset === 'yes') { - // A forced reset / reload starts from a clean slate: drop the previously loaded interval - // storage and canonical index so the incoming page cannot merge into stale intervals. - // Without this, a reload would blank only `state.items`, leaving the old interval behind for - // `ingestPage` to merge the fresh page into. + // Drop the previously loaded interval storage and canonical index so the incoming page cannot + // merge into stale intervals. Driven by `reset`, independent of `keepPreviousItems`. // - // Only a forced reset clears the cache. A first page reached through ordinary shape-change - // detection (e.g. cursor pagination, whose per-page cursor makes every page look like a new - // shape) must PRESERVE the cache so adjacent/overlapping pages merge. Genuine filter/sort - // changes clear the cache separately via `resetState()` in the paginator's own setters. + // A first page reached through ordinary shape-change detection (e.g. cursor pagination, whose + // per-page cursor makes every page look like a new shape) must PRESERVE the cache so + // adjacent/overlapping pages merge. Genuine filter/sort changes clear the cache separately via + // `resetState()` in the paginator's own setters. this.setIntervals([]); this.setActiveInterval(undefined); this._itemIndex.clear(); this.clearIntervalViews(); } - let items: T[] | undefined = undefined; - if (!this.isInitialized) { + let items: T[] | undefined = keepPreviousItems ? this.items : undefined; + if (!this.isInitialized && !keepPreviousItems) { items = (await this.preloadFirstPageFromOfflineDb({ direction, @@ -2428,8 +2433,8 @@ export abstract class BasePaginator { } this.state.next({ ...state, items }); } else if (!silent) { - // Non-first-page, or a keepPreviousItems refresh: surface loading without blanking the list. - // The freshly fetched page is merged into the active interval in postQueryReconcile. + // Non-first-page, or a keepPreviousItems refresh without a forced reset: surface loading without + // blanking the list. The freshly fetched page is merged into the active interval in postQueryReconcile. this.state.partialNext({ isLoading: true }); } diff --git a/test/unit/pagination/paginators/BasePaginator.test.ts b/test/unit/pagination/paginators/BasePaginator.test.ts index 1594818df..69ef7553d 100644 --- a/test/unit/pagination/paginators/BasePaginator.test.ts +++ b/test/unit/pagination/paginators/BasePaginator.test.ts @@ -364,13 +364,52 @@ describe('BasePaginator', () => { expect(paginator.mockClientQuery).toHaveBeenCalledTimes(3); }); - it('keeps hasMoreHead unchanged on a keepPreviousItems first-page refresh (offset)', async () => { - // Regression: hasMoreHead is derived from the start offset only when the first page resets - // the window (isFirstPage && !keepPreviousItems). A keepPreviousItems refresh is isFirstPage - // but does NOT reset the offset, so it must not re-derive hasMoreHead from the grown offset. + it('reload() re-fetches the FIRST page (offset 0) after paginating, not the current page', async () => { + // Regression: executeQuery derived the query shape (which carries `offset`) BEFORE the reset + // restored the initial offset, so reload() on an already-paginated list re-fetched the current + // page (e.g. page 2) and replaced page 1 with it. Probe `this.offset` when the shape is derived + // to prove the reset applies to the OUTGOING request, not just to state after the fact. + const offsetAtQueryShape: number[] = []; + class OffsetProbePaginator extends IncompletePaginator { + getNextQueryShape = vi.fn(() => { + offsetAtQueryShape.push(this.offset); + return defaultNextQueryShape; + }); + } + const paginator = new OffsetProbePaginator({ pageSize: 1 }); + + // Page 1 (offset 0 -> 1) + let nextPromise = paginator.toTail(); + await sleep(0); + paginator.queryResolve({ items: [{ id: 'id1' }] }); + await nextPromise; + expect(paginator.offset).toBe(1); + + // Page 2 (offset 1 -> 2) + nextPromise = paginator.toTail(); + paginator.queryResolve({ items: [{ id: 'id2' }] }); + await nextPromise; + expect(paginator.offset).toBe(2); + + // reload() must derive its request from offset 0, not the paginated 2. + nextPromise = paginator.reload(); + await sleep(0); + expect(offsetAtQueryShape.at(-1)).toBe(0); + + paginator.queryResolve({ items: [{ id: 'id1' }] }); + await nextPromise; + expect(paginator.items).toEqual([{ id: 'id1' }]); + }); + + it('keepPreviousItems + reset resets the window to page 1 while keeping items visible (offset)', async () => { + // `reset` and `keepPreviousItems` are orthogonal: reset re-establishes the window from page 1 + // (offset back to 0, fresh page replaces), while keepPreviousItems only keeps the CURRENT items + // visible during the fetch instead of blanking. Guards: (a) the refresh fetches page 1, not the + // grown offset; (b) items stay visible while the fetch is in flight; (c) the fresh page replaces + // (not merges); (d) hasMoreHead is anchored from the start offset (0), not the grown offset. const paginator = new Paginator({ pageSize: 1 }); - // First page from offset 0 -> head is loaded. + // Page 1 (offset 0 -> 1) let nextPromise = paginator.toTail(); await sleep(0); paginator.queryResolve({ items: [{ id: 'id1' }] }); @@ -378,21 +417,25 @@ describe('BasePaginator', () => { expect(paginator.hasMoreHead).toBe(false); expect(paginator.offset).toBe(1); - // Grow the tail so the offset is well past 0. + // Page 2 (offset 1 -> 2) nextPromise = paginator.toTail(); paginator.queryResolve({ items: [{ id: 'id2' }] }); await nextPromise; expect(paginator.offset).toBe(2); - expect(paginator.hasMoreHead).toBe(false); + expect(paginator.items).toEqual([{ id: 'id1' }, { id: 'id2' }]); - // A non-destructive first-page refresh (isFirstPage via reset, keepPreviousItems) must NOT - // flip hasMoreHead to true off the grown offset (2 > 0) — the window still starts at 0. + // keepPreviousItems reset: the window resets (offset 0) but the current items stay visible. const refreshPromise = paginator.executeQuery({ keepPreviousItems: true, reset: 'yes', }); + expect(paginator.offset).toBe(0); // window reset to the first page + expect(paginator.items).toEqual([{ id: 'id1' }, { id: 'id2' }]); // still visible during the fetch + + // The fresh first page replaces the list (not merged onto the stale window). paginator.queryResolve({ items: [{ id: 'id1' }] }); await refreshPromise; + expect(paginator.items).toEqual([{ id: 'id1' }]); expect(paginator.hasMoreHead).toBe(false); }); From 9fde45c26c949b4b8cebafc45dc851b8d2502e2d Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Fri, 7 Aug 2026 11:20:54 +0200 Subject: [PATCH 2/7] fix: pending item flush update using intermediate state --- src/pagination/paginators/BasePaginator.ts | 14 +++--- .../paginators/BasePaginator.test.ts | 47 +++++++++++++++---- 2 files changed, 44 insertions(+), 17 deletions(-) diff --git a/src/pagination/paginators/BasePaginator.ts b/src/pagination/paginators/BasePaginator.ts index 6ca0e58c0..9101d9208 100644 --- a/src/pagination/paginators/BasePaginator.ts +++ b/src/pagination/paginators/BasePaginator.ts @@ -2406,11 +2406,11 @@ export abstract class BasePaginator { const isFirstPage = this.isFirstPageQuery({ queryShape, reset }); - if (isFirstPage && (!keepPreviousItems || reset === 'yes')) { + if (isFirstPage && !keepPreviousItems) { const state = this.getStateBeforeFirstQuery(); if (reset === 'yes') { - // Drop the previously loaded interval storage and canonical index so the incoming page cannot - // merge into stale intervals. Driven by `reset`, independent of `keepPreviousItems`. + // A forced reset/reload starts from a clean slate: drop the previously loaded interval storage + // and canonical index so the incoming page cannot merge into stale intervals. // // A first page reached through ordinary shape-change detection (e.g. cursor pagination, whose // per-page cursor makes every page look like a new shape) must PRESERVE the cache so @@ -2421,8 +2421,8 @@ export abstract class BasePaginator { this._itemIndex.clear(); this.clearIntervalViews(); } - let items: T[] | undefined = keepPreviousItems ? this.items : undefined; - if (!this.isInitialized && !keepPreviousItems) { + let items: T[] | undefined = undefined; + if (!this.isInitialized) { items = (await this.preloadFirstPageFromOfflineDb({ direction, @@ -2433,8 +2433,8 @@ export abstract class BasePaginator { } this.state.next({ ...state, items }); } else if (!silent) { - // Non-first-page, or a keepPreviousItems refresh without a forced reset: surface loading without - // blanking the list. The freshly fetched page is merged into the active interval in postQueryReconcile. + // Non-first-page, or a keepPreviousItems refresh: surface loading without blanking the list. The + // freshly fetched page is merged into the still-loaded intervals in postQueryReconcile. this.state.partialNext({ isLoading: true }); } diff --git a/test/unit/pagination/paginators/BasePaginator.test.ts b/test/unit/pagination/paginators/BasePaginator.test.ts index 69ef7553d..d31a9818e 100644 --- a/test/unit/pagination/paginators/BasePaginator.test.ts +++ b/test/unit/pagination/paginators/BasePaginator.test.ts @@ -401,12 +401,12 @@ describe('BasePaginator', () => { expect(paginator.items).toEqual([{ id: 'id1' }]); }); - it('keepPreviousItems + reset resets the window to page 1 while keeping items visible (offset)', async () => { - // `reset` and `keepPreviousItems` are orthogonal: reset re-establishes the window from page 1 - // (offset back to 0, fresh page replaces), while keepPreviousItems only keeps the CURRENT items - // visible during the fetch instead of blanking. Guards: (a) the refresh fetches page 1, not the - // grown offset; (b) items stay visible while the fetch is in flight; (c) the fresh page replaces - // (not merges); (d) hasMoreHead is anchored from the start offset (0), not the grown offset. + it('keepPreviousItems + reset is non-destructive: resets to page 1 but keeps the loaded items (offset)', async () => { + // `reset` restores the first-page offset (so the refresh fetches page 1, not the paginated page), + // while `keepPreviousItems` keeps the loaded items AND their storage intact — the fresh page is + // MERGED in, not blanked/collapsed. This is what keeps the channel list stable on a pull-to-refresh + // / reconnect refresh. Guards: (a) offset resets to page 1; (b) items stay visible during the fetch; + // (c) the list is merged (not collapsed to just page 1); (d) hasMoreHead stays anchored at 0. const paginator = new Paginator({ pageSize: 1 }); // Page 1 (offset 0 -> 1) @@ -424,21 +424,48 @@ describe('BasePaginator', () => { expect(paginator.offset).toBe(2); expect(paginator.items).toEqual([{ id: 'id1' }, { id: 'id2' }]); - // keepPreviousItems reset: the window resets (offset 0) but the current items stay visible. + // keepPreviousItems reset: offset resets to page 1, items stay visible during the fetch. const refreshPromise = paginator.executeQuery({ keepPreviousItems: true, reset: 'yes', }); - expect(paginator.offset).toBe(0); // window reset to the first page + expect(paginator.offset).toBe(0); // fetches page 1, not the paginated offset expect(paginator.items).toEqual([{ id: 'id1' }, { id: 'id2' }]); // still visible during the fetch - // The fresh first page replaces the list (not merged onto the stale window). + // The fresh page 1 merges in non-destructively — the loaded list is NOT collapsed to just page 1. paginator.queryResolve({ items: [{ id: 'id1' }] }); await refreshPromise; - expect(paginator.items).toEqual([{ id: 'id1' }]); + expect(paginator.items?.map((i) => i.id).sort()).toEqual(['id1', 'id2']); expect(paginator.hasMoreHead).toBe(false); }); + it('keepPreviousItems + reset keeps the loaded items when an item is ingested mid-refresh (no rebuild-from-empty)', async () => { + // Regression: a keepPreviousItems refresh must NOT clear the item index. Otherwise an item ingested + // concurrently while the refresh query is in flight (e.g. a message.new from the offline-send replay + // on reconnect) rebuilds the list from an empty index and collapses it to just that one item until + // the query resolves. The previously-loaded items must survive the mid-refresh ingest. + const paginator = new Paginator({ pageSize: 3 }); + const loadPromise = paginator.toTail(); + await sleep(0); + paginator.queryResolve({ items: [a, b, c] }); + await loadPromise; + expect(paginator.items).toEqual([a, b, c]); + + // Start a keepPreviousItems refresh (query in flight, not yet resolved). + const refreshPromise = paginator.executeQuery({ + keepPreviousItems: true, + reset: 'yes', + }); + + // A concurrent ingest lands mid-refresh — the already-loaded items must NOT be wiped. + paginator.ingestItem(v); + const idsMidRefresh = paginator.items?.map((item) => item.id) ?? []; + expect(idsMidRefresh).toEqual(expect.arrayContaining(['a', 'b', 'c'])); + + paginator.queryResolve({ items: [a, b, c] }); + await refreshPromise; + }); + it('anchors hasMoreHead from a new start offset when the window is re-established via reset (offset)', async () => { // To start a window mid-list, set the start offset and reset. isFirstPage is true, // getStateBeforeFirstQuery runs, and hasMoreHead is anchored from the (new) start offset. From 78d3a1e76ed9df19d97c07b4ca0ae221ae3db714 Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Fri, 7 Aug 2026 12:26:10 +0200 Subject: [PATCH 3/7] fix: cold start double loading state --- src/pagination/paginators/ChannelPaginator.ts | 7 +++- .../paginators/ChannelPaginator.test.ts | 34 +++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/pagination/paginators/ChannelPaginator.ts b/src/pagination/paginators/ChannelPaginator.ts index 141d8bbf9..a2f49a359 100644 --- a/src/pagination/paginators/ChannelPaginator.ts +++ b/src/pagination/paginators/ChannelPaginator.ts @@ -662,8 +662,13 @@ export class ChannelPaginator extends BasePaginator }); } + // When the sync completes, run the real query — but as a NON-DESTRUCTIVE refresh + // (`keepPreviousItems`) so the channels we already surfaced from the offline DB stay visible while it + // runs. Without this the re-run goes through the first-page reset path and re-preloads from the DB; + // if the sync invalidated the offline query cache (i.e. a channel changed while the app was closed), + // that re-preload returns nothing and the list blanks to a second skeleton before the fresh page lands. offlineDb.syncManager.scheduleSyncStatusChangeCallback(this.id, async () => { - await this.executeQuery(params); + await this.executeQuery({ ...params, keepPreviousItems: true }); }); } diff --git a/test/unit/pagination/paginators/ChannelPaginator.test.ts b/test/unit/pagination/paginators/ChannelPaginator.test.ts index d1aea7874..f65066e58 100644 --- a/test/unit/pagination/paginators/ChannelPaginator.test.ts +++ b/test/unit/pagination/paginators/ChannelPaginator.test.ts @@ -1092,6 +1092,40 @@ describe('ChannelPaginator', () => { expect(queryChannels).toHaveBeenCalledTimes(1); }); + it('does not blank the list on the post-sync re-run when the offline cache was invalidated', async () => { + // Regression: the deferred post-sync re-run must be a NON-DESTRUCTIVE refresh (keepPreviousItems). + // Otherwise it re-preloads from the offline DB; if the sync invalidated the query cache (i.e. a + // channel changed while the app was closed), that re-preload returns nothing and the list blanks + // to a second skeleton before the fresh page lands. + await setUpOfflineDb({ syncStatus: false }); + const cachedChannel = new Channel(client, 'type', 'cached', {}); + getChannelsForQuery.mockResolvedValue([{ channel: cachedChannel.data }]); + vi.spyOn(client, 'hydrateActiveChannels').mockReturnValue([cachedChannel]); + vi.spyOn(client, 'queryChannelsAndHydrate').mockResolvedValue({ + channels: [cachedChannel], + duration: '0.1ms', + }); + const paginator = makePaginator(); + + await paginator.toTail(); // cold start: surface the cached page, defer the query + expect(paginator.items).toStrictEqual([cachedChannel]); + + // The sync invalidates the offline query cache before the deferred re-run fires. + getChannelsForQuery.mockResolvedValue(null); + client.offlineDb!.syncManager.syncStatus = true; + + // Watch for ANY transient blank (items === undefined) while the deferred re-run executes. + let blanked = false; + const unsubscribe = paginator.state.subscribe((next) => { + if (next.items === undefined) blanked = true; + }); + await scheduleSyncStatusChangeCallback.mock.calls[0][1](); + unsubscribe(); + + expect(blanked).toBe(false); + expect(paginator.items).toStrictEqual([cachedChannel]); + }); + it('defers even when nothing is cached and the list is already loaded', async () => { await setUpOfflineDb({ syncStatus: true }); vi.spyOn(client, 'queryChannelsAndHydrate').mockResolvedValue({ From fbdee8c5474deeaf5729c9e3ea91a3ff3e8ad226 Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Fri, 7 Aug 2026 12:51:08 +0200 Subject: [PATCH 4/7] fix: cold start with pending tasks wiping cached channel list --- src/pagination/paginators/ChannelPaginator.ts | 19 +++++++----- .../paginators/ChannelPaginator.test.ts | 31 +++++++++++++++++++ 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/src/pagination/paginators/ChannelPaginator.ts b/src/pagination/paginators/ChannelPaginator.ts index a2f49a359..7b1f4817b 100644 --- a/src/pagination/paginators/ChannelPaginator.ts +++ b/src/pagination/paginators/ChannelPaginator.ts @@ -648,18 +648,21 @@ export class ChannelPaginator extends BasePaginator if (!shouldDeferUntilSynced) return await super.executeQuery(params); if (!this.isInitialized) { - const state = this.getStateBeforeFirstQuery(); const cachedChannels = await this.preloadFirstPageFromOfflineDb({ ...params, queryShape, }); - // `isLoading: false` — nothing is in flight while we wait for the sync, and leaving it set would - // make `canExecuteQuery` reject the query this schedules below. - this.state.next({ - ...state, - isLoading: false, - items: cachedChannels ?? state.items, - }); + if (cachedChannels?.length) { + // Seed via `setItems` (which ingests the page into the interval/index storage), so that the items + // actually appear within the adequate index. + // TODO: Maybe find a better way to do this rather than 2 partialNext invocations + // running. This all happens so fast it should never be noticeable but it's + // a microoptimization. + this.setItems({ valueOrFactory: cachedChannels, isFirstPage: true }); + } + // Nothing is in flight while we wait for the sync; leaving `isLoading` true would make + // `canExecuteQuery` reject the query scheduled below. + this.state.partialNext({ isLoading: false }); } // When the sync completes, run the real query — but as a NON-DESTRUCTIVE refresh diff --git a/test/unit/pagination/paginators/ChannelPaginator.test.ts b/test/unit/pagination/paginators/ChannelPaginator.test.ts index f65066e58..eb2481232 100644 --- a/test/unit/pagination/paginators/ChannelPaginator.test.ts +++ b/test/unit/pagination/paginators/ChannelPaginator.test.ts @@ -1126,6 +1126,37 @@ describe('ChannelPaginator', () => { expect(paginator.items).toStrictEqual([cachedChannel]); }); + it('seeds the preloaded channels into the index so a concurrent ingest does not collapse the list', async () => { + // The cold start preload must SEED the paginator (populate the interval/index), not + // just set the displayed `items`. Otherwise a channel ingested concurrently during the presync + // window (i.e a message.new from the offline-send replay in executePendingTasks) rebuilds the + // list from an empty index and collapses it to just that one channel. + await setUpOfflineDb({ syncStatus: false }); + const a = new Channel(client, 'type', 'a', {}); + const b = new Channel(client, 'type', 'b', {}); + const c = new Channel(client, 'type', 'c', {}); + getChannelsForQuery.mockResolvedValue([{}, {}, {}]); + vi.spyOn(client, 'hydrateActiveChannels').mockReturnValue([a, b, c]); + vi.spyOn(client, 'queryChannelsAndHydrate').mockResolvedValue({ + channels: [a, b, c], + duration: '0.1ms', + }); + const paginator = makePaginator({ filters: {} }); + + await paginator.toTail(); // cold start: preload the cached channels + defer + expect(paginator.items?.map((ch) => ch.cid).sort()).toStrictEqual( + [a.cid, b.cid, c.cid].sort(), + ); + + // A pending-send message.new lands during the defer window (before sync completes). + paginator.ingestItem(a); + + // The list must NOT collapse to just the ingested channel. + expect(paginator.items?.map((ch) => ch.cid).sort()).toStrictEqual( + [a.cid, b.cid, c.cid].sort(), + ); + }); + it('defers even when nothing is cached and the list is already loaded', async () => { await setUpOfflineDb({ syncStatus: true }); vi.spyOn(client, 'queryChannelsAndHydrate').mockResolvedValue({ From 0f0e79e62e138105877e5c6a453d7bed04af3c0d Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Fri, 7 Aug 2026 14:51:29 +0200 Subject: [PATCH 5/7] fix: race condition when the sync process finishes faster than preload --- src/pagination/paginators/ChannelPaginator.ts | 7 +++ .../paginators/ChannelPaginator.test.ts | 48 +++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/src/pagination/paginators/ChannelPaginator.ts b/src/pagination/paginators/ChannelPaginator.ts index 7b1f4817b..36a40d744 100644 --- a/src/pagination/paginators/ChannelPaginator.ts +++ b/src/pagination/paginators/ChannelPaginator.ts @@ -665,6 +665,13 @@ export class ChannelPaginator extends BasePaginator this.state.partialNext({ isLoading: false }); } + // Check if everything is synced up already and if so, just run the actual queryChannels request. + // Otherwise, the sync status change will never fire and so `executeQuery` will never really be + // run. + if (offlineDb.syncManager.syncStatus) { + return await super.executeQuery({ ...params, keepPreviousItems: true }); + } + // When the sync completes, run the real query — but as a NON-DESTRUCTIVE refresh // (`keepPreviousItems`) so the channels we already surfaced from the offline DB stay visible while it // runs. Without this the re-run goes through the first-page reset path and re-preloads from the DB; diff --git a/test/unit/pagination/paginators/ChannelPaginator.test.ts b/test/unit/pagination/paginators/ChannelPaginator.test.ts index eb2481232..affc48ee5 100644 --- a/test/unit/pagination/paginators/ChannelPaginator.test.ts +++ b/test/unit/pagination/paginators/ChannelPaginator.test.ts @@ -1092,6 +1092,54 @@ describe('ChannelPaginator', () => { expect(queryChannels).toHaveBeenCalledTimes(1); }); + it('runs the watching query directly when the sync completes during the preload', async () => { + // This test basically confirms a very intermittent regression that would cause the sync status + // to be changed to true way before the preload/initial population finishes. In that instance, + // we would drop all of the listeners and so the actual query would not fire. + await setUpOfflineDb({ syncStatus: false }); + const cachedChannel = new Channel(client, 'type', 'cached', {}); + // getChannelsForQuery IS the awaited preload: flipping syncStatus here mimics the sync landing + // mid-await (and the sync manager having already drained + cleared its callback map). + getChannelsForQuery.mockImplementation(async () => { + client.offlineDb!.syncManager.syncStatus = true; + return [{ channel: cachedChannel.data }]; + }); + vi.spyOn(client, 'hydrateActiveChannels').mockReturnValue([cachedChannel]); + const queryChannels = vi + .spyOn(client, 'queryChannelsAndHydrate') + .mockResolvedValue({ channels: [cachedChannel], duration: '0.1ms' }); + const paginator = makePaginator(); + + await paginator.toTail(); + + // The watching query ran in THIS call; nothing was left dangling on the already-cleared map. + expect(queryChannels).toHaveBeenCalledTimes(1); + expect(scheduleSyncStatusChangeCallback).not.toHaveBeenCalled(); + // And the preloaded list was not blanked (non-destructive refresh). + expect(paginator.items).toStrictEqual([cachedChannel]); + }); + + it('runs the query directly even when the cache is empty and the sync lands during the preload', async () => { + // Same race, but nothing is cached: the preload returns nothing yet the sync still completes + // mid-await. We must not strand a callback — run the query directly so the (watched) list still lands. + await setUpOfflineDb({ syncStatus: false }); + const fresh = new Channel(client, 'type', 'fresh', {}); + getChannelsForQuery.mockImplementation(async () => { + client.offlineDb!.syncManager.syncStatus = true; + return null; + }); + const queryChannels = vi + .spyOn(client, 'queryChannelsAndHydrate') + .mockResolvedValue({ channels: [fresh], duration: '0.1ms' }); + const paginator = makePaginator({ filters: {} }); + + await paginator.toTail(); + + expect(queryChannels).toHaveBeenCalledTimes(1); + expect(scheduleSyncStatusChangeCallback).not.toHaveBeenCalled(); + expect(paginator.items).toStrictEqual([fresh]); + }); + it('does not blank the list on the post-sync re-run when the offline cache was invalidated', async () => { // Regression: the deferred post-sync re-run must be a NON-DESTRUCTIVE refresh (keepPreviousItems). // Otherwise it re-preloads from the offline DB; if the sync invalidated the query cache (i.e. a From 52fd71c2c411b0b77341cfba7aaeefb12b3b4ddb Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Sat, 8 Aug 2026 02:52:34 +0200 Subject: [PATCH 6/7] fix: remove default boost for new messages --- src/ChannelManager.ts | 7 +--- test/unit/ChannelManager.test.ts | 12 +++--- .../paginators/ChannelPaginator.test.ts | 42 +++++++++++++++++++ 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/src/ChannelManager.ts b/src/ChannelManager.ts index 00896038b..00fec635c 100644 --- a/src/ChannelManager.ts +++ b/src/ChannelManager.ts @@ -180,12 +180,7 @@ const updateLists: EventHandlerPipelineHandler = async ({ // Selected owner: optionally boost then ingest const channelBoost = paginator.getBoost(channel.cid); if ( - [ - 'message.new', - 'notification.message_new', - 'notification.added_to_channel', - 'channel.visible', - ].includes(event.type) && + ['notification.added_to_channel', 'channel.visible'].includes(event.type) && (!channelBoost || channelBoost.seq < paginator.maxBoostSeq) ) { paginator.boost(channel.cid, { seq: paginator.maxBoostSeq + 1 }); diff --git a/test/unit/ChannelManager.test.ts b/test/unit/ChannelManager.test.ts index e498f2553..ae7c8b224 100644 --- a/test/unit/ChannelManager.test.ts +++ b/test/unit/ChannelManager.test.ts @@ -1525,12 +1525,7 @@ describe('ChannelManager', () => { }); }); - it.each([ - 'message.new', - 'notification.message_new', - 'notification.added_to_channel', - 'channel.visible', - ] as EventTypes[])( + it.each(['notification.added_to_channel', 'channel.visible'] as EventTypes[])( 'boosts ingested channel on %s if the item is not already boosted at the top', async (eventType) => { vi.useFakeTimers(); @@ -1597,6 +1592,11 @@ describe('ChannelManager', () => { ); it.each([ + // message events deliberately do NOT boost — a new message bumps last_message_at and the sort + // relocates the channel on its own (see the in-place relocate fix 60566820); boosting them would + // ignore the sort and jump the channel over pinned / archived ones. + 'message.new', + 'notification.message_new', 'channel.updated', 'channel.truncated', 'member.updated', diff --git a/test/unit/pagination/paginators/ChannelPaginator.test.ts b/test/unit/pagination/paginators/ChannelPaginator.test.ts index 3c3b0651e..4b97fad3c 100644 --- a/test/unit/pagination/paginators/ChannelPaginator.test.ts +++ b/test/unit/pagination/paginators/ChannelPaginator.test.ts @@ -1602,5 +1602,47 @@ describe('ChannelPaginator', () => { expect(cids?.[0]).toBe('type:c'); // moved to the head (top) expect(cids).toHaveLength(3); // no duplicates, nothing lost }); + + it('keeps a pinned channel on top when an unpinned channel receives a new message', async () => { + const pinned = new Channel(client, 'type', 'pinned', {}); + const plainA = new Channel(client, 'type', 'plainA', {}); + const plainB = new Channel(client, 'type', 'plainB', {}); + pinned.state.membership = { pinned_at: '2020-01-01T00:00:00.000Z' }; + plainA.state.membership = {}; + plainB.state.membership = {}; + setLastMessageAt(pinned, new Date('2020-01-01T00:00:00.000Z')); // old, but pinned → stays on top + setLastMessageAt(plainA, new Date('2020-03-01T00:00:00.000Z')); // newest unpinned + setLastMessageAt(plainB, new Date('2020-02-01T00:00:00.000Z')); // older unpinned + + const paginator = new ChannelPaginator({ + client, + sort: [ + { field: 'pinned_at', direction: -1 }, + { field: 'last_message_at', direction: -1 }, + ], + paginatorOptions: { + doRequest: () => Promise.resolve({ items: [pinned, plainA, plainB] }), + pageSize: 10, + }, + }); + await paginator.executeQuery({}); + expect(paginator.items?.map((ch) => ch.cid)).toEqual([ + 'type:pinned', + 'type:plainA', + 'type:plainB', + ]); + + // plainB receives a new message → newest last_message_at; re-ingest as updateLists does (no boost). + setLastMessageAt(plainB, new Date('2020-05-01T00:00:00.000Z')); + paginator.ingestItem(plainB); + + // plainB relocates ABOVE plainA (newer message) but stays BELOW the pinned channel — the sort's + // pinned partition holds. A boost would have shoved plainB to index 0, over the pinned channel. + expect(paginator.items?.map((ch) => ch.cid)).toEqual([ + 'type:pinned', + 'type:plainB', + 'type:plainA', + ]); + }); }); }); From 10ca981d45da44f2396e1c713606bc426ca096b4 Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Mon, 10 Aug 2026 13:10:03 +0200 Subject: [PATCH 7/7] fix: remove all default boosting --- src/ChannelManager.ts | 12 ++--- test/unit/ChannelManager.test.ts | 75 +++----------------------------- 2 files changed, 10 insertions(+), 77 deletions(-) diff --git a/src/ChannelManager.ts b/src/ChannelManager.ts index 00fec635c..32dee28bf 100644 --- a/src/ChannelManager.ts +++ b/src/ChannelManager.ts @@ -177,14 +177,10 @@ const updateLists: EventHandlerPipelineHandler = async ({ return; } - // Selected owner: optionally boost then ingest - const channelBoost = paginator.getBoost(channel.cid); - if ( - ['notification.added_to_channel', 'channel.visible'].includes(event.type) && - (!channelBoost || channelBoost.seq < paginator.maxBoostSeq) - ) { - paginator.boost(channel.cid, { seq: paginator.maxBoostSeq + 1 }); - } + // Selected owner: ingest. The manager never boosts by default on any event — the sort is the + // single source of truth for order, so a channel that just became relevant (new message, added, + // unhidden) relocates via its updated sort key. Boosting remains a public per-paginator primitive + // (`paginator.boost`) for integrators to opt into for specific channels (VIP/mention/deep-link). paginator.ingestItem(channel); }); }; diff --git a/test/unit/ChannelManager.test.ts b/test/unit/ChannelManager.test.ts index ae7c8b224..05f6138ad 100644 --- a/test/unit/ChannelManager.test.ts +++ b/test/unit/ChannelManager.test.ts @@ -1525,76 +1525,13 @@ describe('ChannelManager', () => { }); }); - it.each(['notification.added_to_channel', 'channel.visible'] as EventTypes[])( - 'boosts ingested channel on %s if the item is not already boosted at the top', - async (eventType) => { - vi.useFakeTimers(); - const now = new Date('2025-01-01T00:00:00Z'); - vi.setSystemTime(now); - const nowSpy = vi.spyOn(Date, 'now').mockReturnValue(now.getTime()); - - const channelManager = new ChannelManager({ client }); - const ch = makeChannel('messaging:5'); - client.activeChannels[ch.cid] = ch; - - const paginator = new ChannelPaginator({ client }); - const matchesFilterSpy = vi.spyOn(paginator, 'matchesFilter').mockReturnValue(true); - - channelManager.insertPaginator({ paginator }); - channelManager.registerSubscriptions(); - - // @ts-expect-error accessing protected property - expect(paginator.boosts.size).toBe(0); - - client.dispatchEvent({ type: eventType, cid: ch.cid }); - - await vi.waitFor(() => { - // @ts-expect-error accessing protected property - expect(Array.from(paginator.boosts.entries())).toEqual([ - [ch.cid, { seq: 1, until: now.getTime() + 15000 }], - ]); - }); - - client.dispatchEvent({ type: eventType, cid: ch.cid }); - await vi.waitFor(() => { - // already at the top - // @ts-expect-error accessing protected property - expect(Array.from(paginator.boosts.entries())).toEqual([ - [ch.cid, { seq: 1, until: now.getTime() + 15000 }], - ]); - }); - - matchesFilterSpy.mockReturnValue(false); - client.dispatchEvent({ type: eventType, cid: ch.cid }); - - await vi.waitFor(() => { - // @ts-expect-error accessing protected property - expect(Array.from(paginator.boosts.entries())).toEqual([ - [ch.cid, { seq: 1, until: now.getTime() + 15000 }], - ]); - }); - - matchesFilterSpy.mockReturnValue(true); - // @ts-expect-error accessing protected property - paginator._maxBoostSeq = 1000; - client.dispatchEvent({ type: eventType, cid: ch.cid }); - await vi.waitFor(() => { - // some other channel has a higher boost - // @ts-expect-error accessing protected property - expect(Array.from(paginator.boosts.entries())).toEqual([ - [ch.cid, { seq: 1001, until: now.getTime() + 15000 }], - ]); - }); - - nowSpy.mockRestore(); - vi.useRealTimers(); - }, - ); - it.each([ - // message events deliberately do NOT boost — a new message bumps last_message_at and the sort - // relocates the channel on its own (see the in-place relocate fix 60566820); boosting them would - // ignore the sort and jump the channel over pinned / archived ones. + // The manager never boosts by default on ANY event — the sort is the single source of truth for + // order. A new message bumps last_message_at and the sort relocates the channel on its own (see the + // in-place relocate fix 60566820); an added / unhidden channel likewise relocates via its sort key. + // Boosting stays a public per-paginator primitive integrators can opt into (see ChannelManager.updateLists). + 'notification.added_to_channel', + 'channel.visible', 'message.new', 'notification.message_new', 'channel.updated',