Skip to content

Commit 9ad50fd

Browse files
committed
fix(v2): bind an AND-conjoined filter array as a set, not a sequence
The knowledge documents list fingerprinted tagFilters through canonicalJson, which sorts object keys but preserves array order. Each filter compiles to a condition in and(...whereConditions), and AND is commutative, so the same clauses written in a different order select the same documents — and got a different fingerprint, refusing a cursor for a page that was genuinely the next one. Adds unorderedJsonScopePart beside parseUnorderedList: members are canonicalized, de-duplicated, and sorted, so `A AND A` binds like `A` and clause order stops mattering. A non-array or unparseable value still binds by its raw spelling, since that request fails validation anyway. Replaces the route-local canonicalTagFilters, and corrects the claim on canonicalJson that array order only ever costs a restart — for a set-valued filter it costs a spurious 400. Reported by Greptile.
1 parent 2996814 commit 9ad50fd

3 files changed

Lines changed: 58 additions & 20 deletions

File tree

apps/sim/app/api/v2/knowledge/[id]/documents/route.ts

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import {
44
v2ListKnowledgeDocumentsContract,
55
v2UploadKnowledgeDocumentContract,
66
} from '@/lib/api/contracts/v2/knowledge'
7-
import { canonicalJson, cursorScopeKey } from '@/lib/api/cursor-binding'
7+
import { cursorScopeKey, unorderedJsonScopePart } from '@/lib/api/cursor-binding'
88
import {
99
defineV2BodyLifecycleRoute,
1010
defineV2JsonRoute,
@@ -40,21 +40,6 @@ export const revalidate = 0
4040

4141
const MAX_FILE_SIZE = MAX_KNOWLEDGE_DOCUMENT_FILE_SIZE
4242

43-
/**
44-
* Canonical form of `tagFilters` so two equivalent filters differing only in
45-
* key order fingerprint the same. {@link canonicalJson} sorts object keys. An
46-
* unparseable value binds by its raw spelling — the request carrying it is
47-
* about to fail validation anyway.
48-
*/
49-
function canonicalTagFilters(raw: string | undefined): string | undefined {
50-
if (raw === undefined) return undefined
51-
try {
52-
return canonicalJson(JSON.parse(raw))
53-
} catch {
54-
return raw
55-
}
56-
}
57-
5843
/** Every param that changes which documents, in which order, this list returns. */
5944
function documentCursorFilters(
6045
knowledgeBaseId: string,
@@ -65,7 +50,7 @@ function documentCursorFilters(
6550
workspaceId: query.workspaceId,
6651
enabledFilter: query.enabledFilter,
6752
search: query.search,
68-
tagFilters: canonicalTagFilters(query.tagFilters),
53+
tagFilters: unorderedJsonScopePart(query.tagFilters),
6954
})
7055
}
7156

apps/sim/lib/api/cursor-binding.test.ts

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,12 @@
22
* @vitest-environment node
33
*/
44
import { describe, expect, it } from 'vitest'
5-
import { cursorScopeKey, parseUnorderedList, unorderedScopePart } from '@/lib/api/cursor-binding'
5+
import {
6+
cursorScopeKey,
7+
parseUnorderedList,
8+
unorderedJsonScopePart,
9+
unorderedScopePart,
10+
} from '@/lib/api/cursor-binding'
611
import {
712
cursorSortKey,
813
decodeOffsetCursor,
@@ -219,6 +224,29 @@ describe('unordered filter scope parts', () => {
219224
expect(unorderedScopePart('A, B')).toBe(parseUnorderedList('A, B')?.join(','))
220225
})
221226

227+
/**
228+
* Tag filters compile to `and(...)`, so reordering the clauses selects the
229+
* same documents. Binding to the order a caller happened to write them in
230+
* refused a cursor for a page that was genuinely the next one.
231+
*/
232+
it('treats an AND-conjoined filter array as a set', () => {
233+
const ab = '[{"name":"a","value":"1"},{"name":"b","value":"2"}]'
234+
const ba = '[{"name":"b","value":"2"},{"name":"a","value":"1"}]'
235+
236+
expect(unorderedJsonScopePart(ab)).toBe(unorderedJsonScopePart(ba))
237+
expect(unorderedJsonScopePart('[{"name":"a"},{"name":"a"}]')).toBe(
238+
unorderedJsonScopePart('[{"name":"a"}]')
239+
)
240+
expect(unorderedJsonScopePart(ab)).not.toBe(
241+
unorderedJsonScopePart('[{"name":"a","value":"1"}]')
242+
)
243+
})
244+
245+
it('binds an unparseable filter by its raw spelling', () => {
246+
expect(unorderedJsonScopePart('{not json')).toBe('{not json')
247+
expect(unorderedJsonScopePart(undefined)).toBeUndefined()
248+
})
249+
222250
it('still separates genuinely different sets', () => {
223251
expect(cursorScopeKey({ workflowIds: unorderedScopePart('A,B') })).not.toBe(
224252
cursorScopeKey({ workflowIds: unorderedScopePart('A,C') })

apps/sim/lib/api/cursor-binding.ts

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,38 @@ export function parseUnorderedList(raw: string | undefined): string[] | undefine
9494
].sort()
9595
}
9696

97+
/**
98+
* Canonical form of a JSON-encoded filter array whose members are AND-conjoined.
99+
*
100+
* {@link canonicalJson} preserves array order, which is right for a sequence but
101+
* wrong for a set: clauses that compile to `and(...)` select the same rows in any
102+
* order, so binding to the order a caller happened to write them refuses a cursor
103+
* for a page that is genuinely the next one. Members are canonicalized, then
104+
* de-duplicated and sorted — `A AND A` selects what `A` does.
105+
*
106+
* A non-array or unparseable value binds by its raw spelling: the request
107+
* carrying it is about to fail validation anyway.
108+
*/
109+
export function unorderedJsonScopePart(raw: string | undefined): string | undefined {
110+
if (raw === undefined) return undefined
111+
try {
112+
const parsed: unknown = JSON.parse(raw)
113+
if (!Array.isArray(parsed)) return canonicalJson(parsed)
114+
return `[${[...new Set(parsed.map(canonicalJson))].sort().join(',')}]`
115+
} catch {
116+
return raw
117+
}
118+
}
119+
97120
/**
98121
* Deterministic JSON: object keys sorted so two structurally equal values
99122
* serialize identically regardless of the key order they arrived in, and
100123
* `undefined` members dropped so an omitted param and an absent one agree.
101124
*
102-
* Array order is preserved — reordering an `in` list is treated as a different
103-
* filter, which only ever costs a restart.
125+
* Array order is preserved, because an array is a sequence in the general case.
126+
* A filter whose array is really a set must canonicalize it first — see
127+
* {@link parseUnorderedList} and {@link unorderedJsonScopePart} — or equivalent
128+
* queries fingerprint differently and a valid cursor is refused.
104129
*/
105130
export function canonicalJson(value: unknown): string {
106131
if (value instanceof Date) return JSON.stringify(value.toISOString())

0 commit comments

Comments
 (0)