Skip to content

Commit 4c35950

Browse files
committed
fix(knowledge): make a capped listing index the most recent items
Cursor Bugbot, high severity. Both connectors listed ascending by updatedAt and then took the first N indexable rows, so a cap meant "the oldest N". The sharper consequence was the second-order one: an already-indexed item that got edited moved past the cap window, stopped being listed, and — since listingCapped suppresses deletion reconciliation — left a permanently stale document in the knowledge base. Ordering now follows from whether the listing is complete or bounded. Uncapped stays ascending, which is the safe walk: a row updated mid-sync moves toward the end and may be emitted twice, and the engine dedupes on externalId. Capped switches to descending so the cap means "the N most recently active", which is what maxFiles/maxConversations are for. Its own risk — a row updated mid-sync slipping behind the cursor — is already covered, because a capped listing is declared incomplete and the next sync picks the row up. The keyset comparison flips with the order (gt/lt) or page two would repeat page one; both filter builders take the direction explicitly. Verified against the live database: capping to 3 now returns the three most recently active conversations rather than the three oldest.
1 parent 580386b commit 4c35950

4 files changed

Lines changed: 88 additions & 8 deletions

File tree

apps/sim/connectors/sim-conversations/sim-conversations.test.ts

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,23 @@ describe('buildConversationListingFilters', () => {
105105
).toContain('ESCAPE')
106106
})
107107

108+
/** The keyset direction must match ORDER BY — see the files connector's note. */
109+
it('flips the keyset comparison when the listing is descending', () => {
110+
const cursor = { updatedAt: new Date('2026-01-01T00:00:00.000Z'), id: 'mem-1' }
111+
const ascending = conditionsOf({ workspaceId: 'ws-1', prefix: '', cursor })
112+
const descending = conditionsOf({ workspaceId: 'ws-1', prefix: '', cursor, descending: true })
113+
114+
const branches = (nodes: MockCondition[]) =>
115+
nodes
116+
.filter((n) => n.type === 'or')
117+
.flatMap((n) => (n.conditions as MockCondition[]) ?? [])
118+
.flatMap(flattenMockConditions)
119+
120+
expect(branches(ascending).some((n) => n.type === 'gt')).toBe(true)
121+
expect(branches(descending).some((n) => n.type === 'lt')).toBe(true)
122+
expect(branches(descending).some((n) => n.type === 'gt')).toBe(false)
123+
})
124+
108125
it('adds a keyset clause only when paginating', () => {
109126
const first = conditionsOf({ workspaceId: 'ws-1', prefix: '' })
110127
const next = conditionsOf({

apps/sim/connectors/sim-conversations/sim-conversations.ts

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { db } from '@sim/db'
22
import { memory, memorySecretProvenance } from '@sim/db/schema'
3-
import { and, asc, eq, gt, isNull, or, type SQL, sql } from 'drizzle-orm'
3+
import { and, asc, desc, eq, gt, isNull, lt, or, type SQL, sql } from 'drizzle-orm'
44
import { readBoundMemorySecretProvenance } from '@/lib/memory/secret-provenance'
55
import { simConversationsConnectorMeta } from '@/connectors/sim-conversations/meta'
66
import type {
@@ -91,6 +91,8 @@ export function buildConversationListingFilters(args: {
9191
workspaceId: string
9292
prefix: string
9393
cursor?: Cursor
94+
/** Must match the query's ORDER BY, or the keyset walks the wrong way. */
95+
descending?: boolean
9496
}): SQL[] {
9597
const filters: SQL[] = [eq(memory.workspaceId, args.workspaceId), isNull(memory.deletedAt)]
9698

@@ -99,10 +101,11 @@ export function buildConversationListingFilters(args: {
99101
}
100102

101103
if (args.cursor) {
104+
const beyond = args.descending ? lt : gt
102105
filters.push(
103106
or(
104-
gt(memory.updatedAt, args.cursor.updatedAt),
105-
and(eq(memory.updatedAt, args.cursor.updatedAt), gt(memory.id, args.cursor.id))
107+
beyond(memory.updatedAt, args.cursor.updatedAt),
108+
and(eq(memory.updatedAt, args.cursor.updatedAt), beyond(memory.id, args.cursor.id))
106109
) as SQL
107110
)
108111
}
@@ -235,6 +238,13 @@ export const simConversationsConnector: ConnectorConfig = {
235238
const minMessages = parseOptionalPositiveInt(sourceConfig.minMessages) ?? 1
236239
const maxConversations = parseOptionalPositiveInt(sourceConfig.maxConversations) ?? 0
237240

241+
/**
242+
* See the matching note in the files connector: ascending is the safe walk for a
243+
* complete listing, but under a cap it would mean "the oldest N" and would freeze
244+
* an already-indexed conversation the moment it received a new message.
245+
*/
246+
const descending = maxConversations > 0
247+
238248
const rows = await db
239249
.select(CONVERSATION_ROW_COLUMNS)
240250
.from(memory)
@@ -244,10 +254,14 @@ export const simConversationsConnector: ConnectorConfig = {
244254
workspaceId,
245255
prefix: readPrefix(sourceConfig),
246256
cursor: cursor ? decodeCursor(cursor) : undefined,
257+
descending,
247258
})
248259
)
249260
)
250-
.orderBy(asc(memory.updatedAt), asc(memory.id))
261+
.orderBy(
262+
descending ? desc(memory.updatedAt) : asc(memory.updatedAt),
263+
descending ? desc(memory.id) : asc(memory.id)
264+
)
251265
.limit(PAGE_SIZE)
252266

253267
const items: ExternalDocument[] = []

apps/sim/connectors/sim-files/sim-files.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,32 @@ describe('buildFileListingFilters', () => {
184184
expect(columnNames(nodes.filter((n) => n.type === 'isNull'))).toContain('folderId')
185185
})
186186

187+
/**
188+
* The keyset direction must match ORDER BY. Ascending walks forward with `gt`;
189+
* descending (used when a cap is set, so the cap means "most recently active N"
190+
* rather than "oldest N") must walk with `lt` or the second page repeats page one.
191+
*/
192+
it('flips the keyset comparison when the listing is descending', () => {
193+
const cursor = { updatedAt: new Date('2026-01-01T00:00:00.000Z'), id: 'file-1' }
194+
const ascending = orBranches(
195+
conditionsOf({ workspaceId: 'ws-1', folderIds: null, rootOnly: false, cursor })
196+
)
197+
const descending = orBranches(
198+
conditionsOf({
199+
workspaceId: 'ws-1',
200+
folderIds: null,
201+
rootOnly: false,
202+
cursor,
203+
descending: true,
204+
})
205+
)
206+
207+
expect(ascending.some((node) => node.type === 'gt')).toBe(true)
208+
expect(ascending.some((node) => node.type === 'lt')).toBe(false)
209+
expect(descending.some((node) => node.type === 'lt')).toBe(true)
210+
expect(descending.some((node) => node.type === 'gt')).toBe(false)
211+
})
212+
187213
it('adds a keyset clause only when paginating', () => {
188214
const first = conditionsOf({ workspaceId: 'ws-1', folderIds: null, rootOnly: false })
189215
const next = conditionsOf({

apps/sim/connectors/sim-files/sim-files.ts

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { db } from '@sim/db'
22
import { workspaceFiles } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
44
import { getErrorMessage } from '@sim/utils/errors'
5-
import { and, asc, eq, gt, inArray, isNull, or, type SQL } from 'drizzle-orm'
5+
import { and, asc, desc, eq, gt, inArray, isNull, lt, or, type SQL } from 'drizzle-orm'
66
import { getBaseUrl } from '@/lib/core/utils/urls'
77
import { isSupportedFileType, parseBuffer } from '@/lib/file-parsers'
88
import { listWorkspaceFileFolders } from '@/lib/uploads/contexts/workspace/workspace-file-folder-manager'
@@ -143,6 +143,8 @@ export function buildFileListingFilters(args: {
143143
folderIds: string[] | null
144144
rootOnly: boolean
145145
cursor?: Cursor
146+
/** Must match the query's ORDER BY, or the keyset walks the wrong way. */
147+
descending?: boolean
146148
}): SQL[] {
147149
const filters: SQL[] = [
148150
eq(workspaceFiles.workspaceId, args.workspaceId),
@@ -159,12 +161,13 @@ export function buildFileListingFilters(args: {
159161
}
160162

161163
if (args.cursor) {
164+
const beyond = args.descending ? lt : gt
162165
filters.push(
163166
or(
164-
gt(workspaceFiles.updatedAt, args.cursor.updatedAt),
167+
beyond(workspaceFiles.updatedAt, args.cursor.updatedAt),
165168
and(
166169
eq(workspaceFiles.updatedAt, args.cursor.updatedAt),
167-
gt(workspaceFiles.id, args.cursor.id)
170+
beyond(workspaceFiles.id, args.cursor.id)
168171
)
169172
) as SQL
170173
)
@@ -302,6 +305,22 @@ export const simFilesConnector: ConnectorConfig = {
302305
throw error
303306
}
304307

308+
/**
309+
* Ordering follows from whether this listing is complete or bounded.
310+
*
311+
* Uncapped, ascending is the safe walk: a row updated mid-sync moves toward the
312+
* end and may be emitted twice, which the engine dedupes by `externalId`.
313+
*
314+
* Capped, ascending would mean "the oldest N" — and worse, an already-indexed
315+
* file that is edited moves past the cap window, stops being listed, and (because
316+
* `listingCapped` suppresses deletion) leaves a permanently stale document behind.
317+
* Descending makes the cap mean "the N most recently active", which is what a
318+
* `maxFiles` limit is for. Its own risk — a row updated mid-sync slipping behind
319+
* the cursor — is already covered, since a capped listing is declared incomplete
320+
* and the next sync picks the row up.
321+
*/
322+
const descending = maxFiles > 0
323+
305324
const rows = await db
306325
.select(FILE_ROW_COLUMNS)
307326
.from(workspaceFiles)
@@ -312,10 +331,14 @@ export const simFilesConnector: ConnectorConfig = {
312331
folderIds: scope.folderIds,
313332
rootOnly: scope.rootOnly,
314333
cursor: cursor ? decodeCursor(cursor) : undefined,
334+
descending,
315335
})
316336
)
317337
)
318-
.orderBy(asc(workspaceFiles.updatedAt), asc(workspaceFiles.id))
338+
.orderBy(
339+
descending ? desc(workspaceFiles.updatedAt) : asc(workspaceFiles.updatedAt),
340+
descending ? desc(workspaceFiles.id) : asc(workspaceFiles.id)
341+
)
319342
.limit(PAGE_SIZE)
320343

321344
const items: ExternalDocument[] = []

0 commit comments

Comments
 (0)