From 7db7e78d36f2e7e5c4a50aac6e00a6d07835a6bb Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Wed, 16 Sep 2026 23:56:31 +0200 Subject: [PATCH 1/7] feat: message pruning --- docs/instance-configuration.md | 24 + src/configuration/shape.ts | 17 +- src/pagination/paginators/BasePaginator.ts | 248 ++++++++- .../paginators/MessageIntervalPaginator.ts | 33 ++ .../MessagePaginatorWindowCap.test.ts | 471 ++++++++++++++++++ 5 files changed, 789 insertions(+), 4 deletions(-) create mode 100644 test/unit/pagination/paginators/MessagePaginatorWindowCap.test.ts diff --git a/docs/instance-configuration.md b/docs/instance-configuration.md index c7f6e2d2f..696a836d6 100644 --- a/docs/instance-configuration.md +++ b/docs/instance-configuration.md @@ -100,6 +100,29 @@ The per-parent slice wins field by field, so a slice naming only `pageSize` leav `stateThrottleMs` in place. `channel.pinnedMessagesPaginator` is deliberately **not** covered by the shared key: it has a single parent, and it is a different class with its own ordering and endpoint. +#### Bounding the loaded window + +`maxLoadedItems` is the other field that genuinely differs per parent. It caps how many messages a +list keeps loaded: past the cap, the oldest are dropped as new ones arrive, and scrolling back simply +re-fetches them. Unset — the default — means unbounded, which is what you want for an ordinary +conversation. A busy livestream is the case it exists for: + +```ts +client.config.set({ + channel: { messagePaginator: { maxLoadedItems: 200 } }, + thread: { messagePaginator: { maxLoadedItems: 100 } }, +}); +``` + +Three things worth knowing: + +- A value below that list's `pageSize` is raised to it. A cap smaller than a page would prune away the + page a "load older" query had just fetched, and the list would ask for it again. +- Messages the server has not acknowledged — an unsent or failed send — are never dropped, so the + window can sit slightly above the cap while one is pending. +- Only message lists act on it. It is accepted on any paginator path for type reasons, but a paginator + that cannot re-fetch what it dropped ignores it. + `set` deep-merges, so a later call only touches what it names: ```ts @@ -578,6 +601,7 @@ configurable — use a setup function. initialCursor: undefined, // ⚑ construction-only initialOffset: undefined, // ⚑ construction-only lockItemOrder: false, + maxLoadedItems: undefined, // unbounded; see "Bounding the loaded window" below pageSize: 100, // channel message list default retryCount: 0, // i.e. one attempt stateThrottleMs: 500, // ⟳ rebuild — raised from the base's `undefined` diff --git a/src/configuration/shape.ts b/src/configuration/shape.ts index 0436a3cad..439c77b36 100644 --- a/src/configuration/shape.ts +++ b/src/configuration/shape.ts @@ -69,7 +69,16 @@ export type ConfigShape = { readonly [field: string]: ConfigNode }; * to `DeclarativePaginatorConfig` fails the build here until it is described. Same guard as * `INSTANCE_CONFIG_TREE_KEY_PRESENCE` uses for the top-level keys. */ -const PAGINATOR_FIELDS: Record = { +/** + * `maxLoadedItems` is excluded here and documented only under {@link MESSAGE_PAGINATOR_FIELDS}: the + * type allows it on any paginator, but a bounded window only makes sense for a message list — that is + * the only paginator whose declarative config reaches it, and the only one whose pruned items can be + * fetched back by cursor. + */ +const PAGINATOR_FIELDS: Record< + Exclude, + ConfigNode +> = { debounceMs: { description: 'Delay before a queued page request fires, collapsing rapid scrolling into one query.', @@ -129,6 +138,12 @@ const MESSAGE_PAGINATOR_FIELDS: Record< ConfigNode > = { ...PAGINATOR_FIELDS, + maxLoadedItems: { + description: + 'Caps how many messages stay loaded. Past it the oldest are dropped as new ones arrive, and scrolling back re-fetches them. Unset means unbounded. Values below the page size are raised to it.', + kind: 'value', + type: 'number', + }, unreadReferencePolicy: { description: "'snapshot' freezes the unread divider where the user opened the channel until it is explicitly cleared; 'read-state-only' follows the server read state, so the divider moves as messages are marked read.", diff --git a/src/pagination/paginators/BasePaginator.ts b/src/pagination/paginators/BasePaginator.ts index 861356a70..7207b25b1 100644 --- a/src/pagination/paginators/BasePaginator.ts +++ b/src/pagination/paginators/BasePaginator.ts @@ -335,6 +335,7 @@ export type DeclarativePaginatorConfig = { initialCursor?: PaginatorCursor; initialOffset?: number; lockItemOrder?: boolean; + maxLoadedItems?: number; pageSize?: number; retryCount?: number; stateThrottleMs?: number; @@ -400,6 +401,17 @@ export type PaginatorOptions = { * It does not guarantee global stability across interval changes or page jumps. */ lockItemOrder?: boolean; + /** + * Caps how many items this paginator keeps loaded. Once the head window holds more than this, the + * oldest are dropped from it as new ones arrive — the paginator's membership is released and the + * shared store garbage-collects whatever no other holder still references. Pagination stays intact: + * the window re-opens its tailward edge, so scrolling back simply re-fetches. + * + * Unset (the default) means unbounded — see {@link BasePaginator.pruneTailToLimit} for the + * preconditions a prune must meet. Values below `pageSize` are clamped up to it: a cap smaller than + * a page would have the next page pruned away the moment it lands. + */ + maxLoadedItems?: number; /** The item page size to be requested from the server. */ pageSize?: number; /** @@ -414,6 +426,7 @@ export type PaginatorOptions = { type OptionalPaginatorConfigFields = | 'stateThrottleMs' + | 'maxLoadedItems' | 'deriveCursor' | 'doRequest' | 'initialCursor' @@ -519,6 +532,19 @@ export abstract class BasePaginator { * the whole batch produces a single `state.items` emit — independent of state throttling. */ private _windowPublishSuspendDepth = 0; + /** + * Set when a prune ({@link pruneTailToLimit}) moved the tailward edge inward, so the next + * `state.items` publish also carries the pagination fields that move with it instead of emitting + * one of their own — publishing those separately would cost a second notification per prune, the + * one thing the window cap must not do. + * + * A flag rather than the values themselves, on purpose. The same fields are written by the query + * path ({@link postQueryReconcile}), so a cached snapshot draining after a `toTail()` that landed + * inside the same throttle window would republish a pre-merge cursor and re-fetch the page that + * just merged. A flag cannot go stale that way: {@link takePrunedPaginationFields} re-derives from + * the committed interval at publish time, so whoever publishes last publishes the truth. + */ + private _tailEdgePruned = false; /** Set by a suspended op that changed the active window, so {@link batch} publishes once on exit. */ private _suspendedWindowDirty = false; @@ -1031,11 +1057,11 @@ export abstract class BasePaginator { private flushWindowPublish(): void { const items = this.projectActiveWindow(); if (items) { - this.state.partialNext({ items }); + this.state.partialNext({ items, ...this.takePrunedPaginationFields() }); return; } if ((this.state.getLatestValue().items?.length ?? 0) > 0) { - this.state.partialNext({ items: [] }); + this.state.partialNext({ items: [], ...this.takePrunedPaginationFields() }); } } @@ -1057,6 +1083,207 @@ export abstract class BasePaginator { this._viewPublishThrottle?.flush(); } + // --------------------------------------------------------------------------- + // Window cap (pruning) + // --------------------------------------------------------------------------- + + /** + * Whether the consumer currently considers pruning safe. `true` here: a paginator with no UI + * attached, or a UI that never reports, still gets its configured cap. A UI that knows the user is + * reading near the oldest edge overrides this to say "not right now" — see `MessageIntervalPaginator`. + */ + protected get isPruningAllowed(): boolean { + return true; + } + + /** + * Whether `item` can anchor pagination — i.e. the server knows it, so its id is a usable cursor. + * `true` for anything loaded here; subclasses holding locally-created items (an unsent message) + * narrow it. Drives both halves of a prune: such an item may be dropped, and only such an item may + * become the window's new tailward cursor. + */ + protected isPaginationAnchorable(item: T | undefined): boolean { + return !!item; + } + + /** + * The configured window cap, or `undefined` when unbounded. Clamped up to `pageSize`: a cap below a + * page would prune the page that a tailward query had just fetched, and `onEndReached` would fetch + * it again — a loop. Read fresh (both inputs are runtime-configurable), never cached. + */ + protected get effectiveMaxLoadedItems(): number | undefined { + const { maxLoadedItems, pageSize } = this.config; + if (typeof maxLoadedItems !== 'number' || maxLoadedItems <= 0) return undefined; + return Math.max(maxLoadedItems, pageSize); + } + + /** + * Drops the oldest items from `interval` until at most `maxLoadedItems` remain, releasing each from + * the item index so the shared store can garbage-collect whatever no other holder references. + * + * ## Where this is called from, and why that matters + * + * From `ingestItem`, on the interval `insertItemIdIntoInterval` just returned — a **copy that has + * not been committed yet**. So this mutates an object nothing can observe, the single + * `commitInterval` that follows stores and republishes the already-pruned interval, and the window + * emit after it projects from the same interval. A prune therefore costs **no publish of its own**: + * it rides the ones the ingest was going to make anyway. That is the whole design — see + * {@link _tailEdgePruned} for the pagination half. + * + * ## Preconditions + * + * All must hold, or this is a no-op: + * - a cap is configured + * - the consumer has not suspended pruning ({@link isPruningAllowed}) + * - the interval is anchored, is the dataset head, and is the active one. A jumped-away window is + * what the user is reading, and a logical interval has no pagination provenance — nothing dropped + * from it could ever be fetched back + * - it actually holds more than the cap + * + * Assumes cursor pagination: it drops ids without going through `removeItem`, so + * {@link shrinkOffsetAfterRemoval} never runs and an offset-paginated list would drift. That is not + * a reachable configuration today — `maxLoadedItems` only reaches message paginators — so it is + * stated rather than guarded. + * + * Items the server does not know about ({@link isPaginationAnchorable}) are **skipped, not stopped + * at**: an unsent message sorts by the time it was composed, so a cap would otherwise destroy it. + * Skipping leaves the window a few items above the cap at worst, and the next arrival trims it again. + * + * @returns whether anything was pruned. + */ + protected pruneTailToLimit(interval: AnyInterval): boolean { + const limit = this.effectiveMaxLoadedItems; + if (typeof limit === 'undefined' || !this.isPruningAllowed) return false; + + // Anchored intervals only. A logical interval has no pagination provenance, so anything dropped + // from it could never be fetched back. Separated out because it is also the type guard the field + // reads below depend on — `LogicalInterval` declares no `isHead`/`isTail`/`hasMore*`, so folding + // it into the condition below would merely be redundant at runtime while still being required to + // compile. + if (isLogicalInterval(interval)) return false; + + if ( + !interval.isHead || + this._activeIntervalId !== interval.id || + interval.itemIds.length <= limit + ) + return false; + + const ids = interval.itemIds; + // The tail (oldest) edge is index 0 unless the interval stores ids head-first. + const tailEdgeIsFirst = !this.intervalItemIdsAreHeadFirst; + const step = tailEdgeIsFirst ? 1 : -1; + const dropped = new Set(); + + let remaining = ids.length - limit; + for ( + let i = tailEdgeIsFirst ? 0 : ids.length - 1; + remaining > 0 && i >= 0 && i < ids.length; + i += step + ) { + const id = ids[i]; + const item = this._itemIndex.get(id); + if (item && !this.isPaginationAnchorable(item)) continue; + dropped.add(id); + remaining -= 1; + } + + if (!dropped.size) return false; + + // Rebuild rather than splice: skipped local items leave the dropped ids non-contiguous. + interval.itemIds = ids.filter((id) => !dropped.has(id)); + + // One store transaction, so sibling holders of these ids re-project once rather than per id. + // `remove` releases this paginator's reference and the store deletes the content only when the last + // holder lets go, so a pruned message still pinned or held by an open thread survives intact. + this._itemIndex.batch(() => { + for (const id of dropped) this._itemIndex.remove(id); + }); + + // Older messages provably exist on the server again, so re-open the tailward edge — even if this + // window had reached the start. Leaving `isTail` set would also mislead interval merging. + interval.isTail = false; + interval.hasMoreTail = true; + + // Only flag it. The matching `state` fields are derived at publish time, from the interval as + // committed then — see {@link _tailEdgePruned}. + this._tailEdgePruned = true; + + return true; + } + + /** + * The id at one pagination edge of `interval` that is usable as a cursor — the edge-most item the + * server knows about. Not simply `itemIds[0]`: a prune skips locally-created items, so the outermost + * id can be one the server has never seen, and paginating from it would send a client-generated id + * as `id_lt`. + */ + private getPaginationEdgeId(interval: Interval, edge: 'head' | 'tail'): string | null { + const ids = interval.itemIds; + const fromFirst = + edge === 'tail' + ? !this.intervalItemIdsAreHeadFirst + : this.intervalItemIdsAreHeadFirst; + const step = fromFirst ? 1 : -1; + for (let i = fromFirst ? 0 : ids.length - 1; i >= 0 && i < ids.length; i += step) { + const id = ids[i]; + if (this.isPaginationAnchorable(this._itemIndex.get(id))) return id; + } + return null; + } + + /** + * Filters `items` down to the members of `interval`, preserving their given order. Used by the + * order-locked emit path, which composes its array from the previously published one instead of + * re-projecting, and so needs pruned ids taken out explicitly. + */ + private retainIntervalMembers(items: T[], interval: AnyInterval): T[] { + const members = new Set(interval.itemIds); + return items.filter((item) => members.has(this.getItemId(item))); + } + + /** + * The `state` pagination fields that a prune moved, derived from the **committed** active interval + * at call time and cleared. Called by every path that publishes the active window, so they ride that + * publish instead of emitting their own ({@link _tailEdgePruned} explains why this derives rather + * than replaying what the prune saw). + * + * Returns only fields that actually changed, so an unchanged `cursor` keeps its object identity and + * consumers selecting it are not woken. + * + * Two things it deliberately leaves alone: + * - `cursor.headward`, which the prune never touched — carried over verbatim rather than re-derived, + * so a `config.deriveCursor` hook's verdict on the head edge survives. + * - a null tail edge (a window whose every remaining item is locally-created). Publishing `null` + * would read as "tailward exhausted"; the existing cursor still names a message the *server* has, + * so leaving it in place keeps back-pagination working. + */ + private takePrunedPaginationFields(): Partial> { + if (!this._tailEdgePruned) return {}; + this._tailEdgePruned = false; + + if (!this._activeIntervalId) return {}; + const active = this._itemIntervals.get(this._activeIntervalId); + // A logical interval is never pruned, and an absent one has no window left to paginate. + if (!active || isLogicalInterval(active)) return {}; + + const current = this.state.getLatestValue(); + const next: Partial> = {}; + + // From the interval, not hardcoded `true`: a tailward query landing between the prune and this + // publish may have legitimately reached the dataset start again. + if (current.hasMoreTail !== active.hasMoreTail) next.hasMoreTail = active.hasMoreTail; + + if (this.isCursorPagination) { + const tailward = this.getPaginationEdgeId(active, 'tail'); + if (tailward !== null && current.cursor?.tailward !== tailward) { + next.cursor = { headward: current.cursor?.headward, tailward }; + } + } + + return next; + } + /** * {@link _viewPublishThrottle} boundary: apply every interval-view change buffered since the last * flush, then clear the buffer. Independent of the `state.items` window publish, so a view update @@ -2345,6 +2572,12 @@ export abstract class BasePaginator { ); } + // Enforce the window cap BEFORE committing: `targetInterval` is still the uncommitted copy + // `insertItemIdIntoInterval` returned, so the single `commitInterval` below stores and publishes + // the already-pruned interval, and the emit after it projects from that same interval. No-op + // unless a cap is configured — see {@link pruneTailToLimit}. + const prunedNow = this.pruneTailToLimit(targetInterval); + const addedNewInterval = !this._itemIntervals.has(targetInterval.id); this.commitInterval(targetInterval); @@ -2378,7 +2611,15 @@ export abstract class BasePaginator { const nextView = items.slice(); const insertAt = Math.min(originalIndexInState, nextView.length); nextView.splice(insertAt, 0, ingestedItem); - this.state.partialNext({ items: nextView }); + this.state.partialNext({ + // This branch republishes the LAST PUBLISHED array rather than re-projecting, so a prune + // that just emptied ids out of the interval would resurrect them here. Drop them, keeping + // every survivor's relative position — which is the whole point of `lockItemOrder`. + items: prunedNow + ? this.retainIntervalMembers(nextView, targetInterval) + : nextView, + ...this.takePrunedPaginationFields(), + }); } else { /** * Select a correct interval from which the state.items array is derived @@ -2390,6 +2631,7 @@ export abstract class BasePaginator { ? removedItemCoordinates.interval.interval : targetInterval, ), + ...this.takePrunedPaginationFields(), }); } } diff --git a/src/pagination/paginators/MessageIntervalPaginator.ts b/src/pagination/paginators/MessageIntervalPaginator.ts index 3d7f1af72..dfae52241 100644 --- a/src/pagination/paginators/MessageIntervalPaginator.ts +++ b/src/pagination/paginators/MessageIntervalPaginator.ts @@ -260,6 +260,39 @@ export class MessageIntervalPaginator extends BasePaginator< this.flushPendingPublishes(); } + /** + * UI-driven "it is safe to prune right now" signal, set via {@link setPruningAllowed}. Defaults to + * allowed, so a paginator with no UI attached still honours its configured cap. + */ + protected get isPruningAllowed(): boolean { + return this._pruningAllowed; + } + + private _pruningAllowed = true; + + /** + * Tells the paginator whether dropping the oldest loaded messages is currently safe. The SDK calls + * this from its viewability tracking: while the user is reading near the oldest loaded message, + * pruning there would pull content out from under them, so the window is allowed to grow past its + * cap until they scroll back. Only meaningful alongside `maxLoadedItems`. + * + * Deliberately a plain field rather than a `StateStore` — nothing observes it, and a scroll-driven + * signal must not be able to cost a render. + */ + setPruningAllowed = (allowed: boolean) => { + this._pruningAllowed = allowed; + }; + + /** + * A message can anchor pagination once the server has acknowledged it. Narrowing this is what keeps + * an unsent message out of both halves of a prune: it is skipped rather than dropped, and it can + * never become the window's tailward cursor (its id would go out as `id_lt` and mean nothing to the + * server). + */ + protected isPaginationAnchorable(item: LocalMessage | undefined): boolean { + return !!item && this.isServerConfirmedMessage(item); + } + protected get intervalItemIdsAreHeadFirst(): boolean { // Messages are stored in chronological order (created_at asc) within an interval. // Pagination "head" (newest side) is therefore at the END of the `itemIds` array. diff --git a/test/unit/pagination/paginators/MessagePaginatorWindowCap.test.ts b/test/unit/pagination/paginators/MessagePaginatorWindowCap.test.ts new file mode 100644 index 000000000..1f779329d --- /dev/null +++ b/test/unit/pagination/paginators/MessagePaginatorWindowCap.test.ts @@ -0,0 +1,471 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { AnyInterval } from '../../../../src/pagination/paginators/BasePaginator'; +import { MessagePaginator } from '../../../../src/pagination/paginators/MessagePaginator'; +import { setStateThrottlingEnabled } from '../../../../src/pagination/paginators/stateThrottling'; +import { EntityStore } from '../../../../src/entityStore/EntityStore'; +import { formatMessage } from '../../../../src'; +import { generateMsg } from '../../test-utils/generateMessage'; +import { convertDateToTimestamp } from '../../test-utils/time'; +import type { Channel } from '../../../../src/channel'; +import type { StreamChat } from '../../../../src/client'; +import type { LocalMessage, MessageResponse } from '../../../../src/types'; + +// Day `n` of 2020-01 as the created_at, so ids sort the same way they read. +const msg = ( + id: string, + day: number, + overrides: Partial = {}, +): LocalMessage => + formatMessage( + generateMsg({ + id, + cid: 'channel-id', + created_at: convertDateToTimestamp( + `2020-01-${String(day).padStart(2, '0')}T00:00:00.000Z`, + ), + ...overrides, + }) as MessageResponse, + ); + +const ids = (p: MessagePaginator) => p.items?.map((m) => m.id) ?? []; + +/** + * Interval MEMBERSHIP, which is the only thing that distinguishes "this paginator let go of the id" + * from "the content happens to be gone". `getItem` / `state.items` both read through the store and + * would look identical either way. + */ +const memberIds = (p: MessagePaginator): string[] => + (p as unknown as { itemIntervals: AnyInterval[] }).itemIntervals.flatMap( + (i) => i.itemIds, + ); + +const headInterval = (p: MessagePaginator) => + (p as unknown as { itemIntervals: AnyInterval[] }).itemIntervals[0] as AnyInterval & { + isTail: boolean; + hasMoreTail: boolean; + }; + +describe('MessagePaginator — window cap (pruning)', () => { + let store: EntityStore; + let client: StreamChat; + let channel: Channel; + + beforeEach(() => { + store = new EntityStore({ getEntityId: (m) => m.id }); + client = { messageStore: store, user: { id: 'me' } } as unknown as StreamChat; + channel = { + cid: 'channel-id', + getReplies: vi.fn(), + query: vi.fn(), + getClient: () => client, + // postQueryReconcile seeds the unread snapshot from the own-user read state. + state: { read: {} }, + } as unknown as Channel; + }); + + /** An independent channel + store, so two paginators in one test do not share entities. */ + const makeIsolatedChannel = (): Channel => { + const isolatedStore = new EntityStore({ getEntityId: (m) => m.id }); + const isolatedClient = { + messageStore: isolatedStore, + user: { id: 'me' }, + } as unknown as StreamChat; + return { + cid: 'channel-id', + getReplies: vi.fn(), + query: vi.fn(), + getClient: () => isolatedClient, + state: { read: {} }, + } as unknown as Channel; + }; + + afterEach(() => { + vi.restoreAllMocks(); + }); + + /** A paginator whose window is the anchored head with older messages still on the server. */ + const make = (maxLoadedItems?: number) => + new MessagePaginator({ + channel, + paginatorOptions: { maxLoadedItems, pageSize: 3 }, + }); + + const seedHead = (p: MessagePaginator, count: number, { isTail = false } = {}) => { + p.ingestPage({ + page: Array.from({ length: count }, (_, i) => msg(`m${i + 1}`, i + 1)), + isHead: true, + isTail, + setActive: true, + }); + // Discipline: never trust a paginator test that has not asserted the seed landed. + expect(ids(p)).toHaveLength(count); + }; + + describe('the cap itself', () => { + it('drops the oldest as new messages arrive, holding the window at the limit', () => { + const p = make(5); + seedHead(p, 5); + + p.ingestItem(msg('m6', 6)); + expect(ids(p)).toEqual(['m2', 'm3', 'm4', 'm5', 'm6']); + + p.ingestItem(msg('m7', 7)); + expect(ids(p)).toEqual(['m3', 'm4', 'm5', 'm6', 'm7']); + + for (let i = 8; i <= 20; i++) p.ingestItem(msg(`m${i}`, i)); + expect(ids(p)).toHaveLength(5); + expect(ids(p)).toEqual(['m16', 'm17', 'm18', 'm19', 'm20']); + }); + + it('is unbounded when no cap is configured', () => { + const p = make(); + seedHead(p, 5); + for (let i = 6; i <= 20; i++) p.ingestItem(msg(`m${i}`, i)); + expect(ids(p)).toHaveLength(20); + }); + + it('clamps a cap below the page size up to it, so a fetched page cannot be pruned away', () => { + // pageSize is 3; asking for 1 would drop two thirds of every page the moment it landed. + const p = make(1); + seedHead(p, 3); + for (let i = 4; i <= 8; i++) p.ingestItem(msg(`m${i}`, i)); + expect(ids(p)).toHaveLength(3); + }); + }); + + describe('store membership', () => { + it('releases a pruned id, and the store frees it when nothing else holds it', () => { + const p = make(3); + seedHead(p, 3); + expect(store.has('m1')).toBe(true); + + p.ingestItem(msg('m4', 4)); + + expect(memberIds(p)).not.toContain('m1'); + expect(store.has('m1')).toBe(false); + // and the survivors are untouched + expect(memberIds(p)).toEqual(['m2', 'm3', 'm4']); + expect(store.has('m2')).toBe(true); + }); + + it('keeps a pruned message alive in the store while another holder still references it', () => { + const p = make(3); + const sibling = new MessagePaginator({ + channel, + paginatorOptions: { pageSize: 3 }, + }); + seedHead(p, 3); + // A second collection (a thread reply list / the pinned list) holding the same message. + sibling.ingestPage({ + page: [msg('m1', 1)], + isHead: true, + isTail: true, + setActive: true, + }); + expect(sibling.items?.map((m) => m.id)).toEqual(['m1']); + + p.ingestItem(msg('m4', 4)); + + expect(memberIds(p)).not.toContain('m1'); + // Content survives: the prune released one reference, not the entity. + expect(store.has('m1')).toBe(true); + expect(sibling.items?.map((m) => m.id)).toEqual(['m1']); + }); + }); + + describe('pagination after a prune', () => { + it('re-opens the tailward edge and re-points the cursor at the new oldest message', () => { + const p = make(3); + seedHead(p, 3); + + p.ingestItem(msg('m4', 4)); + + expect(p.hasMoreTail).toBe(true); + expect(p.cursor?.tailward).toBe('m2'); + const head = headInterval(p); + expect(head.isTail).toBe(false); + expect(head.hasMoreTail).toBe(true); + }); + + it('re-opens it even when the window had reached the channel start', () => { + const p = make(3); + seedHead(p, 3, { isTail: true }); + expect(p.hasMoreTail).toBe(false); + + p.ingestItem(msg('m4', 4)); + + // Older messages provably exist on the server again — leaving this false would strand the user. + expect(p.hasMoreTail).toBe(true); + expect(p.cursor?.tailward).toBe('m2'); + }); + + it('fetches older messages from the new oldest id', async () => { + const p = make(3); + seedHead(p, 3); + p.ingestItem(msg('m4', 4)); + + (channel.query as ReturnType).mockResolvedValue({ + messages: [generateMsg({ id: 'm1', cid: 'channel-id' })], + }); + + await p.toTail(); + + const [[queryArgs]] = (channel.query as ReturnType).mock.calls; + expect(queryArgs.messages.id_lt).toBe('m2'); + }); + }); + + describe('what it must never prune', () => { + it('skips an unsent message instead of destroying it, and never makes it the cursor', () => { + const p = make(3); + // m1 is the oldest AND is a failed send — it sorts old, so a naive cap would eat it. + p.ingestPage({ + page: [msg('m1', 1, { status: 'failed' }), msg('m2', 2), msg('m3', 3)], + isHead: true, + isTail: false, + setActive: true, + }); + expect(ids(p)).toEqual(['m1', 'm2', 'm3']); + + p.ingestItem(msg('m4', 4)); + + // m2 was dropped instead; the failed send is still there. + expect(ids(p)).toEqual(['m1', 'm3', 'm4']); + expect(store.has('m1')).toBe(true); + // ...and the cursor skipped past it — its id means nothing to the server. + expect(p.cursor?.tailward).toBe('m3'); + + // It survives an extended burst, the window simply sitting a little above the cap. + for (let i = 5; i <= 15; i++) p.ingestItem(msg(`m${i}`, i)); + expect(ids(p)).toContain('m1'); + expect(ids(p)).toHaveLength(3); + }); + + it('does not prune while the user has jumped away from the head', () => { + const p = make(3); + // Head window: the three newest messages. + p.ingestPage({ + page: [msg('m10', 10), msg('m11', 11), msg('m12', 12)], + isHead: true, + isTail: false, + setActive: true, + }); + // A genuinely OLDER window becomes active, as a jump-to-message would make it. + p.ingestPage({ + page: [msg('o1', 1), msg('o2', 2)], + isHead: false, + isTail: true, + setActive: true, + }); + expect(ids(p)).toEqual(['o1', 'o2']); + const before = memberIds(p).length; + + // A new live message lands in the head interval, which is over the cap — and stays there, + // because the window the user is reading is not the one being capped. + p.ingestItem(msg('m13', 13)); + + expect(memberIds(p)).toHaveLength(before + 1); + expect(memberIds(p)).toContain('m10'); + }); + + it('does not prune a logical (live-only) window, which could never be fetched back', () => { + const p = make(2); + // No page ever loaded: live messages land in the logical head, with no pagination provenance. + for (let i = 1; i <= 6; i++) p.ingestItem(msg(`m${i}`, i)); + expect(ids(p)).toHaveLength(6); + }); + }); + + describe('the consumer gate', () => { + it('stops pruning while the consumer says it is unsafe, and resumes after', () => { + const p = make(3); + seedHead(p, 3); + + p.setPruningAllowed(false); + p.ingestItem(msg('m4', 4)); + p.ingestItem(msg('m5', 5)); + expect(ids(p)).toEqual(['m1', 'm2', 'm3', 'm4', 'm5']); + + p.setPruningAllowed(true); + p.ingestItem(msg('m6', 6)); + expect(ids(p)).toEqual(['m4', 'm5', 'm6']); + }); + }); + + describe('with locked item order', () => { + /** + * `lockItemOrder` makes the emit path compose the next array from the LAST PUBLISHED one instead + * of re-projecting from the interval, so a prune on that same ingest would otherwise republish + * the ids it had just dropped. Reachable exactly as the SDK drives it: suspend while the user + * reads old messages, let the window grow past the cap, resume. + */ + it('drops pruned messages from the order-preserved array instead of resurrecting them', () => { + const p = new MessagePaginator({ + channel, + paginatorOptions: { lockItemOrder: true, maxLoadedItems: 5, pageSize: 3 }, + }); + seedHead(p, 5); + + // Grow past the cap with pruning suspended, as scrolling up does. + p.setPruningAllowed(false); + for (let i = 6; i <= 8; i++) p.ingestItem(msg(`m${i}`, i)); + expect(ids(p)).toEqual(['m1', 'm2', 'm3', 'm4', 'm5', 'm6', 'm7', 'm8']); + + // Back at the live edge, then an UPDATE to an already-visible message — the order-locked path. + p.setPruningAllowed(true); + p.ingestItem(msg('m7', 7, { text: 'edited' })); + + expect(ids(p)).toHaveLength(5); + expect(ids(p)).not.toContain('m1'); + expect(ids(p)).not.toContain('m3'); + // the survivors kept their order, and the edit landed + expect(ids(p)).toEqual(['m4', 'm5', 'm6', 'm7', 'm8']); + expect(p.items?.find((m) => m.id === 'm7')?.text).toBe('edited'); + // and the window really is capped, not just the projection + expect(memberIds(p)).toHaveLength(5); + }); + }); + + describe('it costs no publish of its own', () => { + const countPublishes = (p: MessagePaginator) => { + let state = 0; + let views = 0; + // RAW subscribe, not subscribeWithSelector: `partialNext` always allocates a new state object, + // so every publish notifies — including one that only touches `hasMoreTail`/`cursor`. A + // selector keyed on `items` would silently ignore exactly the regression this guards against + // (and `` does select `hasMoreTail`, so such a publish is a real extra render). + p.state.subscribe(() => { + state += 1; + }); + p.intervalViews.subscribeWithSelector( + (s) => ({ items: s.anchoredHead }), + () => { + views += 1; + }, + ); + // subscribeWithSelector fires once immediately; ignore that, as useStateStore would. + state = 0; + views = 0; + return { state: () => state, views: () => views }; + }; + + it('publishes exactly as often with the cap on as with it off', () => { + // Separate stores on purpose: sharing one would make each paginator a sibling subscriber of + // the other's writes, and the resulting cross-notifications would swamp the signal. + const capped = new MessagePaginator({ + channel: makeIsolatedChannel(), + paginatorOptions: { maxLoadedItems: 5, pageSize: 3 }, + }); + const uncapped = new MessagePaginator({ + channel: makeIsolatedChannel(), + paginatorOptions: { pageSize: 3 }, + }); + seedHead(capped, 5); + seedHead(uncapped, 5); + + const cappedCount = countPublishes(capped); + const uncappedCount = countPublishes(uncapped); + + for (let i = 6; i <= 30; i++) { + capped.ingestItem(msg(`m${i}`, i)); + uncapped.ingestItem(msg(`m${i}`, i)); + } + + // The prune rode the ingest's own publishes — it added none. + expect(cappedCount.state()).toBe(uncappedCount.state()); + expect(cappedCount.views()).toBe(uncappedCount.views()); + // ...and it really did prune. + expect(ids(capped)).toHaveLength(5); + expect(ids(uncapped)).toHaveLength(30); + }); + }); + + describe('under the state-publish throttle', () => { + beforeEach(() => { + vi.useFakeTimers(); + vi.setSystemTime(0); + setStateThrottlingEnabled(true); + }); + + afterEach(() => { + setStateThrottlingEnabled(false); + vi.useRealTimers(); + }); + + it('a burst prunes and publishes once, carrying the re-opened tailward edge with it', () => { + const THROTTLE = 200; + const p = new MessagePaginator({ + channel, + paginatorOptions: { maxLoadedItems: 5, pageSize: 3, stateThrottleMs: THROTTLE }, + }); + seedHead(p, 5, { isTail: true }); + + const handler = vi.fn(); + p.state.subscribe(handler); + handler.mockClear(); + + for (let i = 6; i <= 15; i++) p.ingestItem(msg(`m${i}`, i)); // 10 arrivals + // leading edge only so far + expect(handler).toHaveBeenCalledTimes(1); + + vi.advanceTimersByTime(THROTTLE); // single trailing publish + expect(handler).toHaveBeenCalledTimes(2); + + expect(ids(p)).toEqual(['m11', 'm12', 'm13', 'm14', 'm15']); + // The pagination fields rode that same publish rather than emitting on their own. + expect(p.hasMoreTail).toBe(true); + expect(p.cursor?.tailward).toBe('m11'); + }); + + /** + * The prune's pagination fields are published by whichever window publish comes next, which under + * the throttle can be up to a full interval later. The query path writes the same fields. So if + * the prune handed over *values*, a `toTail()` landing in between would be undone at the throttle + * boundary — the cursor rewound to a pre-merge id and the page just merged fetched all over again. + * They are re-derived from the committed interval instead, which is what this pins. + */ + it('does not rewind a cursor that a tailward query moved while its publish was pending', async () => { + const THROTTLE = 200; + const p = new MessagePaginator({ + channel, + paginatorOptions: { maxLoadedItems: 3, pageSize: 3, stateThrottleMs: THROTTLE }, + }); + p.ingestPage({ + page: [msg('m10', 10), msg('m11', 11), msg('m12', 12)], + isHead: true, + isTail: false, + setActive: true, + }); + + // Spend the throttle's leading edge, so the NEXT prune's publish is genuinely deferred. + p.ingestItem(msg('m13', 13)); + expect(p.cursor?.tailward).toBe('m11'); + + // Prunes m11. Nothing is published yet — only the trailing flush is scheduled. + p.ingestItem(msg('m14', 14)); + expect(p.cursor?.tailward).toBe('m11'); + + // The user scrolls up and a full older page merges while that flush is still pending. + (channel.query as ReturnType).mockResolvedValue({ + messages: [1, 2, 3].map((d) => + generateMsg({ + id: `a${d}`, + cid: 'channel-id', + created_at: convertDateToTimestamp(`2020-01-0${d}T00:00:00.000Z`), + }), + ), + }); + await p.toTail(); + + const afterMerge = p.cursor?.tailward; + expect(afterMerge).toBe('a1'); + + vi.advanceTimersByTime(THROTTLE); + + // 'm12' is what the prune itself saw as the new oldest id. Republishing it here would send the + // next `id_lt` back above the page that just merged. + expect(p.cursor?.tailward).not.toBe('m12'); + expect(p.cursor?.tailward).toBe(afterMerge); + }); + }); +}); From 8f53ed466e5b13b0832fa9e2f9ac5a85b67a7f68 Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Thu, 17 Sep 2026 00:24:42 +0200 Subject: [PATCH 2/7] fix: use pruned interval id instead of flag --- src/pagination/paginators/BasePaginator.ts | 41 +++++----- .../MessagePaginatorWindowCap.test.ts | 78 +++++++++++++++++++ 2 files changed, 98 insertions(+), 21 deletions(-) diff --git a/src/pagination/paginators/BasePaginator.ts b/src/pagination/paginators/BasePaginator.ts index 7207b25b1..1338c92c8 100644 --- a/src/pagination/paginators/BasePaginator.ts +++ b/src/pagination/paginators/BasePaginator.ts @@ -533,18 +533,12 @@ export abstract class BasePaginator { */ private _windowPublishSuspendDepth = 0; /** - * Set when a prune ({@link pruneTailToLimit}) moved the tailward edge inward, so the next - * `state.items` publish also carries the pagination fields that move with it instead of emitting - * one of their own — publishing those separately would cost a second notification per prune, the - * one thing the window cap must not do. - * - * A flag rather than the values themselves, on purpose. The same fields are written by the query - * path ({@link postQueryReconcile}), so a cached snapshot draining after a `toTail()` that landed - * inside the same throttle window would republish a pre-merge cursor and re-fetch the page that - * just merged. A flag cannot go stale that way: {@link takePrunedPaginationFields} re-derives from - * the committed interval at publish time, so whoever publishes last publishes the truth. + * The interval whose tailward edge a prune ({@link pruneTailToLimit}) moved inward, so the next + * `state.items` publish also carries the pagination fields that move with it instead of emitting one + * of their own — publishing those separately would cost a second notification per prune, the one + * thing the window cap must not do. */ - private _tailEdgePruned = false; + private _prunedIntervalId?: string; /** Set by a suspended op that changed the active window, so {@link batch} publishes once on exit. */ private _suspendedWindowDirty = false; @@ -1128,7 +1122,7 @@ export abstract class BasePaginator { * `commitInterval` that follows stores and republishes the already-pruned interval, and the window * emit after it projects from the same interval. A prune therefore costs **no publish of its own**: * it rides the ones the ingest was going to make anyway. That is the whole design — see - * {@link _tailEdgePruned} for the pagination half. + * {@link _prunedIntervalId} for the pagination half. * * ## Preconditions * @@ -1205,9 +1199,9 @@ export abstract class BasePaginator { interval.isTail = false; interval.hasMoreTail = true; - // Only flag it. The matching `state` fields are derived at publish time, from the interval as - // committed then — see {@link _tailEdgePruned}. - this._tailEdgePruned = true; + // Only record which interval it was. The matching `state` fields are derived at publish time, + // from this interval as committed then — see {@link _prunedIntervalId}. + this._prunedIntervalId = interval.id; return true; } @@ -1245,13 +1239,15 @@ export abstract class BasePaginator { /** * The `state` pagination fields that a prune moved, derived from the **committed** active interval * at call time and cleared. Called by every path that publishes the active window, so they ride that - * publish instead of emitting their own ({@link _tailEdgePruned} explains why this derives rather + * publish instead of emitting their own ({@link _prunedIntervalId} explains why this derives rather * than replaying what the prune saw). * * Returns only fields that actually changed, so an unchanged `cursor` keeps its object identity and * consumers selecting it are not woken. * - * Two things it deliberately leaves alone: + * Three things it deliberately leaves alone: + * - a window that is no longer the pruned one. `state` tracks the active interval, so once the + * consumer has jumped elsewhere the prune says nothing about what is being published. * - `cursor.headward`, which the prune never touched — carried over verbatim rather than re-derived, * so a `config.deriveCursor` hook's verdict on the head edge survives. * - a null tail edge (a window whose every remaining item is locally-created). Publishing `null` @@ -1259,11 +1255,14 @@ export abstract class BasePaginator { * so leaving it in place keeps back-pagination working. */ private takePrunedPaginationFields(): Partial> { - if (!this._tailEdgePruned) return {}; - this._tailEdgePruned = false; + const prunedIntervalId = this._prunedIntervalId; + if (!prunedIntervalId) return {}; + this._prunedIntervalId = undefined; - if (!this._activeIntervalId) return {}; - const active = this._itemIntervals.get(this._activeIntervalId); + // Whatever is being published now is a window the prune never touched. + if (this._activeIntervalId !== prunedIntervalId) return {}; + + const active = this._itemIntervals.get(prunedIntervalId); // A logical interval is never pruned, and an absent one has no window left to paginate. if (!active || isLogicalInterval(active)) return {}; diff --git a/test/unit/pagination/paginators/MessagePaginatorWindowCap.test.ts b/test/unit/pagination/paginators/MessagePaginatorWindowCap.test.ts index 1f779329d..f90aec12a 100644 --- a/test/unit/pagination/paginators/MessagePaginatorWindowCap.test.ts +++ b/test/unit/pagination/paginators/MessagePaginatorWindowCap.test.ts @@ -467,5 +467,83 @@ describe('MessagePaginator — window cap (pruning)', () => { expect(p.cursor?.tailward).not.toBe('m12'); expect(p.cursor?.tailward).toBe(afterMerge); }); + + /** + * `state` tracks the ACTIVE interval, so a prune of the head says nothing about a window the user + * jumped to in the meantime. Draining onto it would rewrite that window's pagination from a prune + * it had nothing to do with — here, flipping an exhausted tail edge back to a cursor. + */ + it('does not apply a pending prune to a window the consumer jumped to in the meantime', () => { + const THROTTLE = 200; + const p = new MessagePaginator({ + channel, + paginatorOptions: { maxLoadedItems: 3, pageSize: 3, stateThrottleMs: THROTTLE }, + }); + p.ingestPage({ + page: [msg('m10', 10), msg('m11', 11), msg('m12', 12)], + isHead: true, + isTail: false, + setActive: true, + }); + + p.ingestItem(msg('m13', 13)); // spends the leading edge + p.ingestItem(msg('m14', 14)); // prunes m11 — publish deferred to the trailing flush + + // A jump-to-message: an older, already-exhausted window becomes the active one. + p.ingestPage({ + page: [msg('o1', 1), msg('o2', 2)], + isHead: false, + isTail: true, + setActive: true, + }); + expect(ids(p)).toEqual(['o1', 'o2']); + const jumpedCursor = p.cursor?.tailward; + expect(p.hasMoreTail).toBe(false); + + vi.advanceTimersByTime(THROTTLE); + + expect(p.cursor?.tailward).toBe(jumpedCursor); + expect(p.cursor?.tailward).not.toBe('o1'); + expect(p.hasMoreTail).toBe(false); + }); + + /** + * An optimistic (local-user) write flushes this paginator's pending publish early so the send + * renders without the throttle delay — `channel.ts` → `EntityStore.flushSubscribers` → + * `flushState` → `flushPendingPublishes`. That lands mid-window, on a prune whose publish is still + * pending, so it is the path most likely to consume the pending fields without emitting them. + */ + it('carries the pending pagination when an optimistic send flushes the throttle early', () => { + const THROTTLE = 200; + const p = new MessagePaginator({ + channel, + paginatorOptions: { maxLoadedItems: 3, pageSize: 3, stateThrottleMs: THROTTLE }, + }); + p.ingestPage({ + page: [msg('m10', 10), msg('m11', 11), msg('m12', 12)], + isHead: true, + isTail: false, + setActive: true, + }); + + p.ingestItem(msg('m13', 13)); // spends the leading edge; prunes m10 + expect(p.cursor?.tailward).toBe('m11'); + + p.ingestItem(msg('m14', 14)); // prunes m11 — publish deferred to the trailing edge + expect(p.cursor?.tailward).toBe('m11'); + + // The send: ingested, then the store flushes this paginator so it renders immediately. + p.ingestItem(msg('m15', 15)); // prunes m12, still inside the same throttle window + store.flushSubscribers('m15'); + + // The early flush published the pagination as it drained it. + expect(ids(p)).toEqual(['m13', 'm14', 'm15']); + expect(p.cursor?.tailward).toBe('m13'); + expect(p.hasMoreTail).toBe(true); + + // ...and the trailing edge has nothing left to correct. + vi.advanceTimersByTime(THROTTLE); + expect(p.cursor?.tailward).toBe('m13'); + }); }); }); From f767c8415d3a7d7f210fa6588b4161ef0b1bc7b0 Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Thu, 17 Sep 2026 13:18:20 +0200 Subject: [PATCH 3/7] chore: rename to isPruningSuspended --- src/pagination/paginators/BasePaginator.ts | 14 ++++++------ .../paginators/MessageIntervalPaginator.ts | 22 +++++++++---------- .../paginators/BasePaginator.test.ts | 14 ++++++++++++ .../MessagePaginatorWindowCap.test.ts | 8 +++---- 4 files changed, 36 insertions(+), 22 deletions(-) diff --git a/src/pagination/paginators/BasePaginator.ts b/src/pagination/paginators/BasePaginator.ts index 1338c92c8..5189194ad 100644 --- a/src/pagination/paginators/BasePaginator.ts +++ b/src/pagination/paginators/BasePaginator.ts @@ -1082,12 +1082,12 @@ export abstract class BasePaginator { // --------------------------------------------------------------------------- /** - * Whether the consumer currently considers pruning safe. `true` here: a paginator with no UI - * attached, or a UI that never reports, still gets its configured cap. A UI that knows the user is - * reading near the oldest edge overrides this to say "not right now" — see `MessageIntervalPaginator`. + * Whether pruning is on hold. `false` here means a paginator with no UI attached or a UI that + * never reports, still gets its configured cap. For example, a specific UI can ask pruning + * to not happen unless we are near to the tailing edge. */ - protected get isPruningAllowed(): boolean { - return true; + protected get isPruningSuspended(): boolean { + return false; } /** @@ -1128,7 +1128,7 @@ export abstract class BasePaginator { * * All must hold, or this is a no-op: * - a cap is configured - * - the consumer has not suspended pruning ({@link isPruningAllowed}) + * - the consumer has not suspended pruning ({@link isPruningSuspended}) * - the interval is anchored, is the dataset head, and is the active one. A jumped-away window is * what the user is reading, and a logical interval has no pagination provenance — nothing dropped * from it could ever be fetched back @@ -1147,7 +1147,7 @@ export abstract class BasePaginator { */ protected pruneTailToLimit(interval: AnyInterval): boolean { const limit = this.effectiveMaxLoadedItems; - if (typeof limit === 'undefined' || !this.isPruningAllowed) return false; + if (typeof limit === 'undefined' || this.isPruningSuspended) return false; // Anchored intervals only. A logical interval has no pagination provenance, so anything dropped // from it could never be fetched back. Separated out because it is also the type guard the field diff --git a/src/pagination/paginators/MessageIntervalPaginator.ts b/src/pagination/paginators/MessageIntervalPaginator.ts index dfae52241..2e85829d0 100644 --- a/src/pagination/paginators/MessageIntervalPaginator.ts +++ b/src/pagination/paginators/MessageIntervalPaginator.ts @@ -261,26 +261,26 @@ export class MessageIntervalPaginator extends BasePaginator< } /** - * UI-driven "it is safe to prune right now" signal, set via {@link setPruningAllowed}. Defaults to - * allowed, so a paginator with no UI attached still honours its configured cap. + * UI-driven "hold off on pruning" signal, set via {@link setPruningSuspended}. Defaults to not + * suspended, so a paginator with no UI attached still honours its configured cap. */ - protected get isPruningAllowed(): boolean { - return this._pruningAllowed; + protected get isPruningSuspended(): boolean { + return this._pruningSuspended; } - private _pruningAllowed = true; + private _pruningSuspended = false; /** - * Tells the paginator whether dropping the oldest loaded messages is currently safe. The SDK calls - * this from its viewability tracking: while the user is reading near the oldest loaded message, - * pruning there would pull content out from under them, so the window is allowed to grow past its - * cap until they scroll back. Only meaningful alongside `maxLoadedItems`. + * Tells the paginator to hold off on dropping the oldest loaded messages. The SDK calls this from + * its viewability tracking: while the user is reading near the oldest loaded message, pruning there + * would pull content out from under them, so the window is allowed to grow past its cap until they + * scroll back. Only meaningful alongside `maxLoadedItems`. * * Deliberately a plain field rather than a `StateStore` — nothing observes it, and a scroll-driven * signal must not be able to cost a render. */ - setPruningAllowed = (allowed: boolean) => { - this._pruningAllowed = allowed; + setPruningSuspended = (suspended: boolean) => { + this._pruningSuspended = suspended; }; /** diff --git a/test/unit/pagination/paginators/BasePaginator.test.ts b/test/unit/pagination/paginators/BasePaginator.test.ts index c8df2800a..80f53e867 100644 --- a/test/unit/pagination/paginators/BasePaginator.test.ts +++ b/test/unit/pagination/paginators/BasePaginator.test.ts @@ -104,6 +104,20 @@ const y: TestItem = { id: 'y', age: 4, name: 'Y' }; const z: TestItem = { id: 'z', age: 1, name: 'Z' }; describe('BasePaginator', () => { + describe('window cap (pruning)', () => { + /** + * Polarity guard. This getter is the UI's "hold off" latch, and every paginator that actually + * prunes today overrides it — so the BASE value is read by nothing, and a flipped default would + * pass the whole window-cap suite while silently disabling pruning for any paginator that does + * not override. It was in fact inverted once, by a rename. + */ + it('does not suspend pruning by default, so a paginator with no UI still honours its cap', () => { + const paginator = new Paginator(); + // @ts-expect-error accessing protected property + expect(paginator.isPruningSuspended).toBe(false); + }); + }); + describe('constructor', () => { it('initiates with the defaults', () => { const paginator = new Paginator(); diff --git a/test/unit/pagination/paginators/MessagePaginatorWindowCap.test.ts b/test/unit/pagination/paginators/MessagePaginatorWindowCap.test.ts index f90aec12a..76897343a 100644 --- a/test/unit/pagination/paginators/MessagePaginatorWindowCap.test.ts +++ b/test/unit/pagination/paginators/MessagePaginatorWindowCap.test.ts @@ -281,12 +281,12 @@ describe('MessagePaginator — window cap (pruning)', () => { const p = make(3); seedHead(p, 3); - p.setPruningAllowed(false); + p.setPruningSuspended(true); p.ingestItem(msg('m4', 4)); p.ingestItem(msg('m5', 5)); expect(ids(p)).toEqual(['m1', 'm2', 'm3', 'm4', 'm5']); - p.setPruningAllowed(true); + p.setPruningSuspended(false); p.ingestItem(msg('m6', 6)); expect(ids(p)).toEqual(['m4', 'm5', 'm6']); }); @@ -307,12 +307,12 @@ describe('MessagePaginator — window cap (pruning)', () => { seedHead(p, 5); // Grow past the cap with pruning suspended, as scrolling up does. - p.setPruningAllowed(false); + p.setPruningSuspended(true); for (let i = 6; i <= 8; i++) p.ingestItem(msg(`m${i}`, i)); expect(ids(p)).toEqual(['m1', 'm2', 'm3', 'm4', 'm5', 'm6', 'm7', 'm8']); // Back at the live edge, then an UPDATE to an already-visible message — the order-locked path. - p.setPruningAllowed(true); + p.setPruningSuspended(false); p.ingestItem(msg('m7', 7, { text: 'edited' })); expect(ids(p)).toHaveLength(5); From 93ac0a3eb5d46271412cf7653665ceaba6e98d45 Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Thu, 17 Sep 2026 15:52:36 +0200 Subject: [PATCH 4/7] fix: update comment --- src/pagination/paginators/BasePaginator.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pagination/paginators/BasePaginator.ts b/src/pagination/paginators/BasePaginator.ts index 5189194ad..2b61c61c2 100644 --- a/src/pagination/paginators/BasePaginator.ts +++ b/src/pagination/paginators/BasePaginator.ts @@ -533,10 +533,10 @@ export abstract class BasePaginator { */ private _windowPublishSuspendDepth = 0; /** - * The interval whose tailward edge a prune ({@link pruneTailToLimit}) moved inward, so the next - * `state.items` publish also carries the pagination fields that move with it instead of emitting one - * of their own — publishing those separately would cost a second notification per prune, the one - * thing the window cap must not do. + * The interval a prune just shortened. A prune also moves `hasMoreTail` and `cursor` and those come with + * the next `state.items` publish rather than emitting their own. We use the id instead of direct values + * because that publish can be a throttle interval late, so {@link takePrunedPaginationFields} rereads them + * when it fires. */ private _prunedIntervalId?: string; /** Set by a suspended op that changed the active window, so {@link batch} publishes once on exit. */ From a3027974967925b9f41d68d24b1acb9794176786 Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Thu, 17 Sep 2026 16:07:53 +0200 Subject: [PATCH 5/7] fix: update doc --- src/pagination/paginators/BasePaginator.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/pagination/paginators/BasePaginator.ts b/src/pagination/paginators/BasePaginator.ts index 2b61c61c2..fd56440ec 100644 --- a/src/pagination/paginators/BasePaginator.ts +++ b/src/pagination/paginators/BasePaginator.ts @@ -1091,10 +1091,11 @@ export abstract class BasePaginator { } /** - * Whether `item` can anchor pagination — i.e. the server knows it, so its id is a usable cursor. - * `true` for anything loaded here; subclasses holding locally-created items (an unsent message) - * narrow it. Drives both halves of a prune: such an item may be dropped, and only such an item may - * become the window's new tailward cursor. + * Whether the server knows about `item`. That is what makes its id safe to send as a cursor, and it + * is all this checks — the id itself is never looked at. `true` here for anything present; + * subclasses holding locally-created items (an unsent message) narrow it. Both halves of a prune + * ask: an item the server knows may be dropped, and only such an item may become the window's new + * tailward cursor. */ protected isPaginationAnchorable(item: T | undefined): boolean { return !!item; From 4decfb649e0d9f0c790759ef932b9e8ad6e9f3f1 Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Thu, 17 Sep 2026 16:18:00 +0200 Subject: [PATCH 6/7] fix: upd comment --- src/pagination/paginators/BasePaginator.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/pagination/paginators/BasePaginator.ts b/src/pagination/paginators/BasePaginator.ts index fd56440ec..d16573759 100644 --- a/src/pagination/paginators/BasePaginator.ts +++ b/src/pagination/paginators/BasePaginator.ts @@ -1270,8 +1270,9 @@ export abstract class BasePaginator { const current = this.state.getLatestValue(); const next: Partial> = {}; - // From the interval, not hardcoded `true`: a tailward query landing between the prune and this - // publish may have legitimately reached the dataset start again. + // Read this off the interval instead of assuming the prune's `true`. Pruning reopens the + // tailward (older) edge, but this publish can run a throttle interval later and a `toTail()` in + // between may have loaded the rest of the history - so there may be nothing older left after all. if (current.hasMoreTail !== active.hasMoreTail) next.hasMoreTail = active.hasMoreTail; if (this.isCursorPagination) { From fff84e02e0a63627b6a42a7e14ae1dab8e07c996 Mon Sep 17 00:00:00 2001 From: Ivan Sekovanikj Date: Thu, 17 Sep 2026 16:52:43 +0200 Subject: [PATCH 7/7] fix: rename to something better --- src/pagination/paginators/BasePaginator.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/pagination/paginators/BasePaginator.ts b/src/pagination/paginators/BasePaginator.ts index d16573759..31230502e 100644 --- a/src/pagination/paginators/BasePaginator.ts +++ b/src/pagination/paginators/BasePaginator.ts @@ -535,7 +535,7 @@ export abstract class BasePaginator { /** * The interval a prune just shortened. A prune also moves `hasMoreTail` and `cursor` and those come with * the next `state.items` publish rather than emitting their own. We use the id instead of direct values - * because that publish can be a throttle interval late, so {@link takePrunedPaginationFields} rereads them + * because that publish can be a throttle interval late, so {@link consumePendingPrunePaginationState} rereads them * when it fires. */ private _prunedIntervalId?: string; @@ -1051,11 +1051,11 @@ export abstract class BasePaginator { private flushWindowPublish(): void { const items = this.projectActiveWindow(); if (items) { - this.state.partialNext({ items, ...this.takePrunedPaginationFields() }); + this.state.partialNext({ items, ...this.consumePendingPrunePaginationState() }); return; } if ((this.state.getLatestValue().items?.length ?? 0) > 0) { - this.state.partialNext({ items: [], ...this.takePrunedPaginationFields() }); + this.state.partialNext({ items: [], ...this.consumePendingPrunePaginationState() }); } } @@ -1255,7 +1255,7 @@ export abstract class BasePaginator { * would read as "tailward exhausted"; the existing cursor still names a message the *server* has, * so leaving it in place keeps back-pagination working. */ - private takePrunedPaginationFields(): Partial> { + private consumePendingPrunePaginationState(): Partial> { const prunedIntervalId = this._prunedIntervalId; if (!prunedIntervalId) return {}; this._prunedIntervalId = undefined; @@ -2619,7 +2619,7 @@ export abstract class BasePaginator { items: prunedNow ? this.retainIntervalMembers(nextView, targetInterval) : nextView, - ...this.takePrunedPaginationFields(), + ...this.consumePendingPrunePaginationState(), }); } else { /** @@ -2632,7 +2632,7 @@ export abstract class BasePaginator { ? removedItemCoordinates.interval.interval : targetInterval, ), - ...this.takePrunedPaginationFields(), + ...this.consumePendingPrunePaginationState(), }); } }